From bc8425f7ab4b1b024f49c506cb6622ca1b347a22 Mon Sep 17 00:00:00 2001 From: josephkmh Date: Thu, 15 Dec 2022 14:27:32 -0600 Subject: [PATCH 01/11] pull up field toggling callback to parent --- .../connection/CatalogTree/CatalogSection.tsx | 53 ++++++++++--------- .../CatalogTree/StreamFieldTable.tsx | 27 ++-------- 2 files changed, 31 insertions(+), 49 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx index 0b8792a09d4d..72a936ea37bc 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx @@ -100,28 +100,34 @@ const CatalogSectionInner: React.FC = ({ const numberOfFieldsInStream = Object.keys(streamNode?.stream?.jsonSchema?.properties).length ?? 0; - const onSelectedFieldsUpdate = (selectedFields: SelectedFieldInfo[]) => { - updateStreamWithConfig({ - selectedFields, - fieldSelectionEnabled: true, - }); - }; - - // All fields in a stream are implicitly selected. When deselecting the first one, we also need to explicitly select the rest. - const onFirstFieldDeselected = (fieldPath: string[]) => { - const allOtherFields = fields.filter((field: SyncSchemaField) => !isEqual(field.path, fieldPath)) ?? []; - const selectedFields: SelectedFieldInfo[] = allOtherFields.map((field) => ({ fieldPath: field.path })); - updateStreamWithConfig({ - selectedFields, - fieldSelectionEnabled: true, - }); - }; + const onSelectedFieldsUpdate = (fieldPath: string[], isSelected: boolean) => { + const previouslySelectedFields = config?.selectedFields || []; - const onAllFieldsSelected = () => { - updateStreamWithConfig({ - selectedFields: [], - fieldSelectionEnabled: false, - }); + if (!config?.fieldSelectionEnabled && !isSelected) { + // All fields in a stream are implicitly selected. When deselecting the first one, we also need to explicitly select the rest. + const allOtherFields = fields.filter((field: SyncSchemaField) => !isEqual(field.path, fieldPath)) ?? []; + const selectedFields: SelectedFieldInfo[] = allOtherFields.map((field) => ({ fieldPath: field.path })); + updateStreamWithConfig({ + selectedFields, + fieldSelectionEnabled: true, + }); + } else if (isSelected && previouslySelectedFields.length === numberOfFieldsInStream - 1) { + // In this case we are selecting the only unselected field + updateStreamWithConfig({ + selectedFields: [], + fieldSelectionEnabled: false, + }); + } else if (isSelected) { + updateStreamWithConfig({ + selectedFields: [...previouslySelectedFields, { fieldPath }], + fieldSelectionEnabled: true, + }); + } else { + updateStreamWithConfig({ + selectedFields: previouslySelectedFields.filter((f) => !isEqual(f.fieldPath, fieldPath)) || [], + fieldSelectionEnabled: true, + }); + } }; const pkRequired = config?.destinationSyncMode === DestinationSyncMode.append_dedup; @@ -210,12 +216,9 @@ const CatalogSectionInner: React.FC = ({ diff --git a/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx b/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx index 7220f4fceaa4..d4fd2daea569 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx @@ -2,7 +2,7 @@ import isEqual from "lodash/isEqual"; import React, { useCallback } from "react"; import { SyncSchemaField, SyncSchemaFieldObject } from "core/domain/catalog"; -import { AirbyteStreamConfiguration, SelectedFieldInfo } from "core/request/AirbyteClient"; +import { AirbyteStreamConfiguration } from "core/request/AirbyteClient"; import { FieldHeader } from "./FieldHeader"; import { FieldRow } from "./FieldRow"; @@ -13,43 +13,22 @@ import { TreeRowWrapper } from "./TreeRowWrapper"; interface StreamFieldTableProps { config: AirbyteStreamConfiguration | undefined; onCursorSelect: (cursorPath: string[]) => void; - onFirstFieldDeselected: (fieldName: string[]) => void; onPkSelect: (pkPath: string[]) => void; - onSelectedFieldsUpdate: (selectedFields: SelectedFieldInfo[]) => void; - onAllFieldsSelected: () => void; + handleFieldToggle: (fieldPath: string[], isSelected: boolean) => void; shouldDefineCursor: boolean; shouldDefinePk: boolean; syncSchemaFields: SyncSchemaField[]; - numberOfSelectableFields: number; } export const StreamFieldTable: React.FC = ({ config, onCursorSelect, - onFirstFieldDeselected, onPkSelect, - onSelectedFieldsUpdate, - onAllFieldsSelected, + handleFieldToggle, shouldDefineCursor, shouldDefinePk, syncSchemaFields, - numberOfSelectableFields, }) => { - const handleFieldToggle = (fieldPath: string[], isSelected: boolean) => { - const previouslySelectedFields = config?.selectedFields || []; - - if (!config?.fieldSelectionEnabled && !isSelected) { - onFirstFieldDeselected(fieldPath); - } else if (isSelected && previouslySelectedFields.length === numberOfSelectableFields - 1) { - // In this case we are selecting the only unselected field - onAllFieldsSelected(); - } else if (isSelected) { - onSelectedFieldsUpdate([...previouslySelectedFields, { fieldPath }]); - } else { - onSelectedFieldsUpdate(previouslySelectedFields.filter((f) => !isEqual(f.fieldPath, fieldPath)) || []); - } - }; - const isFieldSelected = useCallback( (field: SyncSchemaField): boolean => { // All fields are implicitly selected if field selection is disabled From 8fc0e41f3cc707d80c4938e4cc5594b1eb9d7746 Mon Sep 17 00:00:00 2001 From: josephkmh Date: Thu, 22 Dec 2022 14:46:30 -0600 Subject: [PATCH 02/11] add unit tests to pk & cursor selection --- .../connection/CatalogTree/CatalogSection.tsx | 45 ++-- .../connection/CatalogTree/FieldRow.tsx | 21 +- .../CatalogTree/streamConfigHelpers/index.ts | 1 + .../streamConfigHelpers.test.ts | 194 ++++++++++++++++++ .../streamConfigHelpers.ts | 98 +++++++++ airbyte-webapp/src/locales/en.json | 2 + 6 files changed, 342 insertions(+), 19 deletions(-) create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/index.ts create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts create mode 100644 airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts diff --git a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx index 72a936ea37bc..cbb36bdfc349 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx @@ -16,12 +16,13 @@ import { } from "core/request/AirbyteClient"; import { useDestinationNamespace } from "hooks/connection/useDestinationNamespace"; import { useConnectionFormService } from "hooks/services/ConnectionForm/ConnectionFormService"; -import { equal, naturalComparatorBy } from "utils/objects"; +import { naturalComparatorBy } from "utils/objects"; import { ConnectionFormValues, SUPPORTED_MODES } from "views/Connection/ConnectionForm/formConfig"; import styles from "./CatalogSection.module.scss"; import { CatalogTreeTableRow } from "./next/CatalogTreeTableRow"; import { StreamDetailsPanel } from "./next/StreamDetailsPanel/StreamDetailsPanel"; +import { updatePrimaryKey, addFieldToPrimaryKey, updateCursorField } from "./streamConfigHelpers/streamConfigHelpers"; import { StreamFieldTable } from "./StreamFieldTable"; import { StreamHeader } from "./StreamHeader"; import { flatten, getPathType } from "./utils"; @@ -47,6 +48,8 @@ const CatalogSectionInner: React.FC = ({ }) => { const isNewStreamsTableEnabled = process.env.REACT_APP_NEW_STREAMS_TABLE ?? false; + const numberOfFieldsInStream = Object.keys(streamNode?.stream?.jsonSchema?.properties).length ?? 0; + const { destDefinition: { supportedDestinationSyncModes }, } = useConnectionFormService(); @@ -75,30 +78,42 @@ const CatalogSectionInner: React.FC = ({ const onPkSelect = useCallback( (pkPath: string[]) => { - let newPrimaryKey: string[][]; - - if (config?.primaryKey?.find((pk) => equal(pk, pkPath))) { - newPrimaryKey = config.primaryKey.filter((key) => !equal(key, pkPath)); - } else { - newPrimaryKey = [...(config?.primaryKey ?? []), pkPath]; + if (!config) { + return; } - updateStreamWithConfig({ primaryKey: newPrimaryKey }); + const updatedConfig = addFieldToPrimaryKey(config, pkPath, numberOfFieldsInStream); + + updateStreamWithConfig(updatedConfig); }, - [config?.primaryKey, updateStreamWithConfig] + [config, updateStreamWithConfig, numberOfFieldsInStream] ); const onCursorSelect = useCallback( - (cursorField: string[]) => updateStreamWithConfig({ cursorField }), - [updateStreamWithConfig] + (cursorField: string[]) => { + if (!config) { + return; + } + + const updatedConfig = updateCursorField(config, cursorField, numberOfFieldsInStream); + + updateStreamWithConfig(updatedConfig); + }, + [config, numberOfFieldsInStream, updateStreamWithConfig] ); const onPkUpdate = useCallback( - (newPrimaryKey: string[][]) => updateStreamWithConfig({ primaryKey: newPrimaryKey }), - [updateStreamWithConfig] - ); + (newPrimaryKey: string[][]) => { + if (!config) { + return; + } - const numberOfFieldsInStream = Object.keys(streamNode?.stream?.jsonSchema?.properties).length ?? 0; + const updatedConfig = updatePrimaryKey(config, newPrimaryKey, numberOfFieldsInStream); + + updateStreamWithConfig(updatedConfig); + }, + [config, updateStreamWithConfig, numberOfFieldsInStream] + ); const onSelectedFieldsUpdate = (fieldPath: string[], isSelected: boolean) => { const previouslySelectedFields = config?.selectedFields || []; diff --git a/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx index f2ec7d64fceb..88e6e8edaa71 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx @@ -1,4 +1,4 @@ -import React, { memo } from "react"; +import React, { memo, useCallback } from "react"; import { FormattedMessage } from "react-intl"; import styled from "styled-components"; @@ -54,18 +54,31 @@ const FieldRowInner: React.FC = ({ const isCursor = equal(config?.cursorField, field.path); const isPrimaryKey = !!config?.primaryKey?.some((p) => equal(p, field.path)); const isNestedField = SyncSchemaFieldObject.isNestedField(field); + const isDisabled = isCursor || isPrimaryKey || isNestedField; + const renderDisabledReasonMessage = useCallback(() => { + if (isNestedField) { + return ; + } + if (isPrimaryKey) { + return ; + } + if (isCursor) { + return ; + } + return null; + }, [isCursor, isPrimaryKey, isNestedField, field.path]); return ( <> {isColumnSelectionEnabled && ( - {!isNestedField && ( + {!isDisabled && ( onToggleFieldSelected(field.path, !isSelected)} /> )} - {isNestedField && ( + {isDisabled && ( }> - + {renderDisabledReasonMessage()} )} diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/index.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/index.ts new file mode 100644 index 000000000000..cc0dfabc8a19 --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/index.ts @@ -0,0 +1 @@ +export * from "./streamConfigHelpers"; diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts new file mode 100644 index 000000000000..be5d97060f0b --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts @@ -0,0 +1,194 @@ +import { AirbyteStreamConfiguration } from "core/request/AirbyteClient"; + +import { addFieldToPrimaryKey, updatePrimaryKey, updateCursorField } from "./streamConfigHelpers"; + +const mockStreamConfiguration: AirbyteStreamConfiguration = { + fieldSelectionEnabled: false, + selectedFields: [], + selected: true, + syncMode: "full_refresh", + destinationSyncMode: "overwrite", +}; + +describe(`${updateCursorField.name}`, () => { + it("updates the cursor field when field selection is disabled", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + fieldSelectionEnabled: false, + selectedFields: [], + }; + + const newStreamConfiguration = updateCursorField(mockConfig, ["new_cursor"], 3); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + cursorField: ["new_cursor"], + }) + ); + }); + + it("updates the cursor field when it is the only unselected field", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], + }; + + const newStreamConfiguration = updateCursorField(mockConfig, ["new_cursor"], 3); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + cursorField: ["new_cursor"], + fieldSelectionEnabled: false, + selectedFields: [], + }) + ); + }); + + it("updates the cursor field when it is one of many unselected fields", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], + }; + + const newStreamConfiguration = updateCursorField(mockConfig, ["new_cursor"], 100); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + cursorField: ["new_cursor"], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }, { fieldPath: ["new_cursor"] }], + }) + ); + }); +}); + +describe(`${updatePrimaryKey.name}`, () => { + it("updates the primary key field", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [["original_pk"]], + }; + + const newStreamConfiguration = updatePrimaryKey(mockConfig, [["new_pk"]], 3); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + primaryKey: [["new_pk"]], + }) + ); + }); + + it("updates the primary key field when field selection is enabled", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [["field_one"]], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: ["field_one"] }], + }; + + const newStreamConfiguration = updatePrimaryKey(mockConfig, [["field_two"]], 3); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + primaryKey: [["field_two"]], + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], + fieldSelectionEnabled: true, + }) + ); + }); + + it("adds the selected primary key to selectedFields", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], + }; + + const newStreamConfiguration = updatePrimaryKey(mockConfig, [["field_three"]], 5); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + primaryKey: [["field_three"]], + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }, { fieldPath: ["field_three"] }], + fieldSelectionEnabled: true, + }) + ); + }); + + it("disables field selection when selected primary key is the last unselected field", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [["field_one"]], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], + }; + + const newStreamConfiguration = updatePrimaryKey(mockConfig, [["field_three"]], 3); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + primaryKey: [["field_three"]], + selectedFields: [], + fieldSelectionEnabled: false, + }) + ); + }); +}); + +describe(`${addFieldToPrimaryKey.name}`, () => { + it("adds a new field to the composite primary key", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [["first_pk_field"]], + }; + + const newStreamConfiguration = addFieldToPrimaryKey(mockConfig, ["second_pk_field"], 3); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + primaryKey: [["first_pk_field"], ["second_pk_field"]], + }) + ); + }); + + it("adds the new primary key field to selectedFields when field selection is enabled", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [["field_one"]], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], + }; + + const newStreamConfiguration = addFieldToPrimaryKey(mockConfig, ["field_three"], 5); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + primaryKey: [["field_one"], ["field_three"]], + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }, { fieldPath: ["field_three"] }], + fieldSelectionEnabled: true, + }) + ); + }); + + it("disables field selection when selected primary key is the last unselected field", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [["field_one"]], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], + }; + + const newStreamConfiguration = addFieldToPrimaryKey(mockConfig, ["field_three"], 3); + + expect(newStreamConfiguration).toEqual( + expect.objectContaining({ + primaryKey: [["field_one"], ["field_three"]], + selectedFields: [], + fieldSelectionEnabled: false, + }) + ); + }); +}); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts new file mode 100644 index 000000000000..01326c8e2104 --- /dev/null +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts @@ -0,0 +1,98 @@ +import isEqual from "lodash/isEqual"; + +import { AirbyteStreamConfiguration } from "core/request/AirbyteClient"; + +/** + * Updates teh cursor field in AirbyteStreamConfiguration + */ +export const updateCursorField = ( + config: AirbyteStreamConfiguration, + selectedCursorField: string[], + numberOfFieldsInStream: number +): Partial => { + const previouslySelectedFields = config?.selectedFields || []; + + // If field selection is enabled, we need to be sure the new cursor is also selected + if (config?.fieldSelectionEnabled && !previouslySelectedFields.find((field) => isEqual(field, selectedCursorField))) { + if (previouslySelectedFields.length === numberOfFieldsInStream - 1) { + return { cursorField: selectedCursorField, selectedFields: [], fieldSelectionEnabled: false }; + } + return { + fieldSelectionEnabled: true, + selectedFields: [...previouslySelectedFields, { fieldPath: selectedCursorField }], + cursorField: selectedCursorField, + }; + } + return { cursorField: selectedCursorField }; +}; + +/** + * Overwrites the entire primaryKey value in AirbyteStreamConfiguration. + */ +export const updatePrimaryKey = ( + config: AirbyteStreamConfiguration, + compositePrimaryKey: string[][], + numberOfFieldsInStream: number +): Partial => { + const previouslySelectedFields = config?.selectedFields || []; + + // If field selection is enabled, we need to be sure each fieldPath in the new primary key is also selected + if ( + config?.fieldSelectionEnabled && + !compositePrimaryKey.some((fieldPath) => previouslySelectedFields.find((field) => isEqual(field, fieldPath))) + ) { + if (previouslySelectedFields.length === numberOfFieldsInStream - 1) { + return { primaryKey: compositePrimaryKey, selectedFields: [], fieldSelectionEnabled: false }; + } + return { + fieldSelectionEnabled: true, + selectedFields: [...previouslySelectedFields, ...compositePrimaryKey.map((fieldPath) => ({ fieldPath }))], + primaryKey: compositePrimaryKey, + }; + } + + return { + primaryKey: compositePrimaryKey, + }; +}; + +/** + * Adds a single fieldPath to the composite primaryKey + */ +export const addFieldToPrimaryKey = ( + config: AirbyteStreamConfiguration, + fieldPath: string[], + numberOfFieldsInStream: number +): Partial => { + const fieldIsSelected = !config?.primaryKey?.find((pk) => isEqual(pk, fieldPath)); + const previouslySelectedFields = config?.selectedFields || []; + let newPrimaryKey: string[][]; + + if (!fieldIsSelected) { + newPrimaryKey = config.primaryKey?.filter((key) => !isEqual(key, fieldPath)) ?? []; + } else { + newPrimaryKey = [...(config?.primaryKey ?? []), fieldPath]; + } + + // If field selection is enabled, we need to be sure the new fieldPath is also selected + if ( + fieldIsSelected && + config?.fieldSelectionEnabled && + !previouslySelectedFields.find((field) => isEqual(field, fieldPath)) + ) { + debugger; + if (previouslySelectedFields.length === numberOfFieldsInStream - 1) { + debugger; + return { primaryKey: newPrimaryKey, selectedFields: [], fieldSelectionEnabled: false }; + } + return { + fieldSelectionEnabled: true, + selectedFields: [...previouslySelectedFields, { fieldPath }], + primaryKey: newPrimaryKey, + }; + } + + return { + primaryKey: newPrimaryKey, + }; +}; diff --git a/airbyte-webapp/src/locales/en.json b/airbyte-webapp/src/locales/en.json index 5cd36830f8ee..602ad7d5b6eb 100644 --- a/airbyte-webapp/src/locales/en.json +++ b/airbyte-webapp/src/locales/en.json @@ -218,6 +218,8 @@ "form.nullable": "Nullable", "form.field.sync": "Sync", "form.field.sync.nestedFieldTooltip": "This field will be synced if \"{fieldName}\" is selected", + "form.field.sync.cursorFieldTooltip": "The cursor field cannot be deselected", + "form.field.sync.primaryKeyTooltip": "The primary key cannot be deselected", "form.field.name": "Field name", "form.field.dataType": "Data type", "form.field.destinationName": "Destination name", From 6d41589699d891f9c063f962652580d2e1dda7b9 Mon Sep 17 00:00:00 2001 From: josephkmh Date: Tue, 3 Jan 2023 13:15:48 +0100 Subject: [PATCH 03/11] only prevent pk & cursor deselection if enabled --- .../src/components/connection/CatalogTree/FieldRow.tsx | 3 ++- .../CatalogTree/streamConfigHelpers/streamConfigHelpers.ts | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx index 88e6e8edaa71..44a9954dc11b 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx @@ -54,7 +54,8 @@ const FieldRowInner: React.FC = ({ const isCursor = equal(config?.cursorField, field.path); const isPrimaryKey = !!config?.primaryKey?.some((p) => equal(p, field.path)); const isNestedField = SyncSchemaFieldObject.isNestedField(field); - const isDisabled = isCursor || isPrimaryKey || isNestedField; + const isDisabled = (isCursorEnabled && isCursor) || (isPrimaryKeyEnabled && isPrimaryKey) || isNestedField; + const renderDisabledReasonMessage = useCallback(() => { if (isNestedField) { return ; diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts index 01326c8e2104..d5b2f1a95ff0 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts @@ -80,9 +80,7 @@ export const addFieldToPrimaryKey = ( config?.fieldSelectionEnabled && !previouslySelectedFields.find((field) => isEqual(field, fieldPath)) ) { - debugger; if (previouslySelectedFields.length === numberOfFieldsInStream - 1) { - debugger; return { primaryKey: newPrimaryKey, selectedFields: [], fieldSelectionEnabled: false }; } return { From b53c2a806cdb44f998600ca5d095cc3664702ca5 Mon Sep 17 00:00:00 2001 From: josephkmh Date: Thu, 5 Jan 2023 23:42:57 +0100 Subject: [PATCH 04/11] fix source defined pk and cursors --- .../connection/CatalogTree/CatalogSection.tsx | 4 ++- .../connection/CatalogTree/FieldRow.tsx | 34 ++++++++++++------- .../CatalogTree/StreamFieldTable.tsx | 22 +++++++----- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx index 4b788595aa39..ebc50c002a70 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx @@ -236,7 +236,9 @@ const CatalogSectionInner: React.FC = ({ onCursorSelect={onCursorSelect} onPkSelect={onPkSelect} handleFieldToggle={onSelectedFieldsUpdate} - shouldDefinePk={shouldDefinePk} + primaryKeyIndexerType={pkType} + cursorIndexerType={cursorType} + shouldDefinePrimaryKey={shouldDefinePk} shouldDefineCursor={shouldDefineCursor} /> diff --git a/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx index fd356e54922b..f13519dbd0d4 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx @@ -15,18 +15,20 @@ import { equal } from "utils/objects"; import { useTranslateDataType } from "utils/useTranslateDataType"; import DataTypeCell from "./DataTypeCell"; -import { pathDisplayName } from "./PathPopout"; +import { IndexerType, pathDisplayName } from "./PathPopout"; import { NameContainer, SyncCheckboxContainer } from "./styles"; interface FieldRowProps { - isPrimaryKeyEnabled: boolean; - isCursorEnabled: boolean; + cursorIndexerType: IndexerType; + primaryKeyIndexerType: IndexerType; isSelected: boolean; onPrimaryKeyChange: (pk: string[]) => void; onCursorChange: (cs: string[]) => void; onToggleFieldSelected: (fieldPath: string[], isSelected: boolean) => void; field: SyncSchemaField; config: AirbyteStreamConfiguration | undefined; + shouldDefinePrimaryKey: boolean; + shouldDefineCursor: boolean; } const FirstCell = styled(Cell)` @@ -43,9 +45,11 @@ const FieldRowInner: React.FC = ({ onToggleFieldSelected, field, config, - isCursorEnabled, - isPrimaryKeyEnabled, + cursorIndexerType, + primaryKeyIndexerType, isSelected, + shouldDefinePrimaryKey, + shouldDefineCursor, }) => { const isColumnSelectionEnabled = useExperiment("connection.columnSelection", false); const dataType = useTranslateDataType(field); @@ -54,30 +58,32 @@ const FieldRowInner: React.FC = ({ const isCursor = equal(config?.cursorField, field.path); const isPrimaryKey = !!config?.primaryKey?.some((p) => equal(p, field.path)); const isNestedField = SyncSchemaFieldObject.isNestedField(field); - const isDisabled = (isCursorEnabled && isCursor) || (isPrimaryKeyEnabled && isPrimaryKey) || isNestedField; + // The indexer type tells us whether a cursor or pk is user-defined, source-defined or not required (null) + const fieldSelectionDisabled = + (cursorIndexerType !== null && isCursor) || (primaryKeyIndexerType !== null && isPrimaryKey) || isNestedField; const renderDisabledReasonMessage = useCallback(() => { if (isNestedField) { return ; } - if (isPrimaryKey) { + if (primaryKeyIndexerType !== null && isPrimaryKey) { return ; } - if (isCursor) { + if (cursorIndexerType !== null && isCursor) { return ; } return null; - }, [isCursor, isPrimaryKey, isNestedField, field.path]); + }, [isCursor, isPrimaryKey, isNestedField, field.path, cursorIndexerType, primaryKeyIndexerType]); return ( <> {isColumnSelectionEnabled && ( - {!isDisabled && ( + {!fieldSelectionDisabled && ( onToggleFieldSelected(field.path, !isSelected)} /> )} - {isDisabled && ( + {fieldSelectionDisabled && ( }> {renderDisabledReasonMessage()} @@ -96,9 +102,11 @@ const FieldRowInner: React.FC = ({ )} {dataType} - {isCursorEnabled && onCursorChange(field.path)} />} - {isPrimaryKeyEnabled && onPrimaryKeyChange(field.path)} />} + {shouldDefineCursor && onCursorChange(field.path)} />} + + + {shouldDefinePrimaryKey && onPrimaryKeyChange(field.path)} />} {field.cleanedName} diff --git a/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx b/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx index d4fd2daea569..3f06cec5e208 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx @@ -1,12 +1,12 @@ import isEqual from "lodash/isEqual"; import React, { useCallback } from "react"; -import { SyncSchemaField, SyncSchemaFieldObject } from "core/domain/catalog"; +import { SyncSchemaField } from "core/domain/catalog"; import { AirbyteStreamConfiguration } from "core/request/AirbyteClient"; import { FieldHeader } from "./FieldHeader"; import { FieldRow } from "./FieldRow"; -import { pathDisplayName } from "./PathPopout"; +import { IndexerType, pathDisplayName } from "./PathPopout"; import styles from "./StreamFieldTable.module.scss"; import { TreeRowWrapper } from "./TreeRowWrapper"; @@ -15,9 +15,11 @@ interface StreamFieldTableProps { onCursorSelect: (cursorPath: string[]) => void; onPkSelect: (pkPath: string[]) => void; handleFieldToggle: (fieldPath: string[], isSelected: boolean) => void; - shouldDefineCursor: boolean; - shouldDefinePk: boolean; + cursorIndexerType: IndexerType; + primaryKeyIndexerType: IndexerType; syncSchemaFields: SyncSchemaField[]; + shouldDefinePrimaryKey: boolean; + shouldDefineCursor: boolean; } export const StreamFieldTable: React.FC = ({ @@ -25,9 +27,11 @@ export const StreamFieldTable: React.FC = ({ onCursorSelect, onPkSelect, handleFieldToggle, - shouldDefineCursor, - shouldDefinePk, + cursorIndexerType, + primaryKeyIndexerType, syncSchemaFields, + shouldDefinePrimaryKey, + shouldDefineCursor, }) => { const isFieldSelected = useCallback( (field: SyncSchemaField): boolean => { @@ -53,12 +57,14 @@ export const StreamFieldTable: React.FC = ({ ))} From 31c95c88e2737ccfc1e27b958450a7b99b6d778f Mon Sep 17 00:00:00 2001 From: josephkmh Date: Fri, 6 Jan 2023 17:07:15 +0100 Subject: [PATCH 05/11] add comment --- .../CatalogTree/streamConfigHelpers/streamConfigHelpers.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts index d5b2f1a95ff0..190ca85ca0e7 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts @@ -41,6 +41,8 @@ export const updatePrimaryKey = ( config?.fieldSelectionEnabled && !compositePrimaryKey.some((fieldPath) => previouslySelectedFields.find((field) => isEqual(field, fieldPath))) ) { + // If the fieldPath being added to the primaryKey is the only unselected field, + // we can actually just disable field selection, since all fields are now selected. if (previouslySelectedFields.length === numberOfFieldsInStream - 1) { return { primaryKey: compositePrimaryKey, selectedFields: [], fieldSelectionEnabled: false }; } From 27218cd0b09275a112960ec5babdd7446f504ecb Mon Sep 17 00:00:00 2001 From: josephkmh Date: Fri, 6 Jan 2023 17:10:32 +0100 Subject: [PATCH 06/11] fix typo --- .../CatalogTree/streamConfigHelpers/streamConfigHelpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts index 190ca85ca0e7..ac68ae5603b8 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts @@ -3,7 +3,7 @@ import isEqual from "lodash/isEqual"; import { AirbyteStreamConfiguration } from "core/request/AirbyteClient"; /** - * Updates teh cursor field in AirbyteStreamConfiguration + * Updates the cursor field in AirbyteStreamConfiguration */ export const updateCursorField = ( config: AirbyteStreamConfiguration, From ea5900e47bce41941407e20c156f5b7b1b511b1a Mon Sep 17 00:00:00 2001 From: josephkmh Date: Mon, 9 Jan 2023 12:25:37 +0100 Subject: [PATCH 07/11] prevent rendering new react node --- .../connection/CatalogTree/FieldRow.tsx | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx b/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx index f13519dbd0d4..f1904ed1a755 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/FieldRow.tsx @@ -80,14 +80,19 @@ const FieldRowInner: React.FC = ({ {isColumnSelectionEnabled && ( - {!fieldSelectionDisabled && ( - onToggleFieldSelected(field.path, !isSelected)} /> - )} - {fieldSelectionDisabled && ( - }> - {renderDisabledReasonMessage()} - - )} + onToggleFieldSelected(field.path, !isSelected)} + /> + } + > + {renderDisabledReasonMessage()} + )} From 9cbe65a0470bc69e2b5b6c6e6307f81d95f74bc0 Mon Sep 17 00:00:00 2001 From: josephkmh Date: Mon, 9 Jan 2023 16:40:15 +0100 Subject: [PATCH 08/11] fix fieldSelection behavior with one unselected field --- .../connection/CatalogTree/CatalogSection.tsx | 8 +- .../streamConfigHelpers.test.ts | 346 ++++++++++++------ .../streamConfigHelpers.ts | 77 ++-- 3 files changed, 281 insertions(+), 150 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx index 83b54acabc93..ddb73b3aaaf3 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx @@ -22,7 +22,11 @@ import { ConnectionFormValues, SUPPORTED_MODES } from "views/Connection/Connecti import styles from "./CatalogSection.module.scss"; import { CatalogTreeTableRow } from "./next/CatalogTreeTableRow"; import { StreamDetailsPanel } from "./next/StreamDetailsPanel/StreamDetailsPanel"; -import { updatePrimaryKey, addFieldToPrimaryKey, updateCursorField } from "./streamConfigHelpers/streamConfigHelpers"; +import { + updatePrimaryKey, + toggleFieldInPrimaryKey, + updateCursorField, +} from "./streamConfigHelpers/streamConfigHelpers"; import { StreamFieldTable } from "./StreamFieldTable"; import { StreamHeader } from "./StreamHeader"; import { flatten, getPathType } from "./utils"; @@ -82,7 +86,7 @@ const CatalogSectionInner: React.FC = ({ return; } - const updatedConfig = addFieldToPrimaryKey(config, pkPath, numberOfFieldsInStream); + const updatedConfig = toggleFieldInPrimaryKey(config, pkPath, numberOfFieldsInStream); updateStreamWithConfig(updatedConfig); }, diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts index be5d97060f0b..75e4855daffa 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts @@ -1,6 +1,11 @@ import { AirbyteStreamConfiguration } from "core/request/AirbyteClient"; -import { addFieldToPrimaryKey, updatePrimaryKey, updateCursorField } from "./streamConfigHelpers"; +import { + mergeFieldPathArrays, + toggleFieldInPrimaryKey, + updatePrimaryKey, + updateCursorField, +} from "./streamConfigHelpers"; const mockStreamConfiguration: AirbyteStreamConfiguration = { fieldSelectionEnabled: false, @@ -10,6 +15,23 @@ const mockStreamConfiguration: AirbyteStreamConfiguration = { destinationSyncMode: "overwrite", }; +const FIELD_ONE = ["field_one"]; +const FIELD_TWO = ["field_two"]; +const FIELD_THREE = ["field_three"]; + +describe(`${mergeFieldPathArrays.name}`, () => { + it("merges two arrays of fieldPaths without duplicates", () => { + const arr1 = [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }]; + const arr2 = [{ fieldPath: FIELD_TWO }, { fieldPath: FIELD_THREE }]; + + expect(mergeFieldPathArrays(arr1, arr2)).toEqual([ + { fieldPath: FIELD_ONE }, + { fieldPath: FIELD_TWO }, + { fieldPath: FIELD_THREE }, + ]); + }); +}); + describe(`${updateCursorField.name}`, () => { it("updates the cursor field when field selection is disabled", () => { const mockConfig: AirbyteStreamConfiguration = { @@ -18,49 +40,76 @@ describe(`${updateCursorField.name}`, () => { selectedFields: [], }; - const newStreamConfiguration = updateCursorField(mockConfig, ["new_cursor"], 3); + const newStreamConfiguration = updateCursorField(mockConfig, FIELD_ONE, 3); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - cursorField: ["new_cursor"], - }) - ); + expect(newStreamConfiguration).toEqual({ + cursorField: FIELD_ONE, + }); }); + describe("when fieldSelection is active", () => { + it("adds the cursor to selectedFields", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; - it("updates the cursor field when it is the only unselected field", () => { - const mockConfig: AirbyteStreamConfiguration = { - ...mockStreamConfiguration, - fieldSelectionEnabled: true, - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], - }; + const newStreamConfiguration = updateCursorField(mockConfig, FIELD_THREE, 100); - const newStreamConfiguration = updateCursorField(mockConfig, ["new_cursor"], 3); + expect(newStreamConfiguration).toEqual({ + cursorField: FIELD_THREE, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }, { fieldPath: FIELD_THREE }], + }); + }); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - cursorField: ["new_cursor"], - fieldSelectionEnabled: false, - selectedFields: [], - }) - ); - }); + it("updates the cursor field when only one other field is unselected", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; - it("updates the cursor field when it is one of many unselected fields", () => { - const mockConfig: AirbyteStreamConfiguration = { - ...mockStreamConfiguration, - fieldSelectionEnabled: true, - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], - }; + const newStreamConfiguration = updateCursorField(mockConfig, FIELD_ONE, 3); + + expect(newStreamConfiguration).toEqual({ + cursorField: FIELD_ONE, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }); + }); + + it("updates the cursor field when it is one of many unselected fields", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; - const newStreamConfiguration = updateCursorField(mockConfig, ["new_cursor"], 100); + const newStreamConfiguration = updateCursorField(mockConfig, ["new_cursor"], 100); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ + expect(newStreamConfiguration).toEqual({ cursorField: ["new_cursor"], fieldSelectionEnabled: true, - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }, { fieldPath: ["new_cursor"] }], - }) - ); + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }, { fieldPath: ["new_cursor"] }], + }); + }); + + it("disables field selection when the selected cursor is the only unselected field", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; + + const newStreamConfiguration = updateCursorField(mockConfig, FIELD_THREE, 3); + + expect(newStreamConfiguration).toEqual({ + cursorField: FIELD_THREE, + fieldSelectionEnabled: false, + selectedFields: [], + }); + }); }); }); @@ -68,127 +117,184 @@ describe(`${updatePrimaryKey.name}`, () => { it("updates the primary key field", () => { const mockConfig: AirbyteStreamConfiguration = { ...mockStreamConfiguration, - primaryKey: [["original_pk"]], + primaryKey: [FIELD_ONE], }; - const newStreamConfiguration = updatePrimaryKey(mockConfig, [["new_pk"]], 3); + const newStreamConfiguration = updatePrimaryKey(mockConfig, [FIELD_TWO], 3); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - primaryKey: [["new_pk"]], - }) - ); + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_TWO], + }); }); - it("updates the primary key field when field selection is enabled", () => { - const mockConfig: AirbyteStreamConfiguration = { - ...mockStreamConfiguration, - primaryKey: [["field_one"]], - fieldSelectionEnabled: true, - selectedFields: [{ fieldPath: ["field_one"] }], - }; + describe("when fieldSelection is active", () => { + it("adds each piece of the composite primary key to selectedFields", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [FIELD_ONE], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }], + }; - const newStreamConfiguration = updatePrimaryKey(mockConfig, [["field_two"]], 3); + const newStreamConfiguration = updatePrimaryKey(mockConfig, [FIELD_TWO, FIELD_THREE], 100); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - primaryKey: [["field_two"]], - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_TWO, FIELD_THREE], + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }, { fieldPath: FIELD_THREE }], fieldSelectionEnabled: true, - }) - ); - }); + }); + }); - it("adds the selected primary key to selectedFields", () => { - const mockConfig: AirbyteStreamConfiguration = { - ...mockStreamConfiguration, - primaryKey: [], - fieldSelectionEnabled: true, - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], - }; + it("replaces the primary key when many other field are unselected", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; - const newStreamConfiguration = updatePrimaryKey(mockConfig, [["field_three"]], 5); + const newStreamConfiguration = updatePrimaryKey(mockConfig, [FIELD_THREE], 100); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - primaryKey: [["field_three"]], - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }, { fieldPath: ["field_three"] }], + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_THREE], + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }, { fieldPath: FIELD_THREE }], fieldSelectionEnabled: true, - }) - ); - }); + }); + }); - it("disables field selection when selected primary key is the last unselected field", () => { - const mockConfig: AirbyteStreamConfiguration = { - ...mockStreamConfiguration, - primaryKey: [["field_one"]], - fieldSelectionEnabled: true, - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], - }; + it("replaces the primary key when only one other field is unselected", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [FIELD_ONE], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; + + const newStreamConfiguration = updatePrimaryKey(mockConfig, [FIELD_TWO], 3); + + expect(newStreamConfiguration).toEqual({ + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + primaryKey: [FIELD_TWO], + }); + }); + + it("disables field selection when the selected primary key is the last unselected field", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }], + }; - const newStreamConfiguration = updatePrimaryKey(mockConfig, [["field_three"]], 3); + const newStreamConfiguration = updatePrimaryKey(mockConfig, [FIELD_TWO, FIELD_THREE], 3); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - primaryKey: [["field_three"]], + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_TWO, FIELD_THREE], selectedFields: [], fieldSelectionEnabled: false, - }) - ); + }); + }); + + it("disables field selection when part of the selected primary key is the last unselected field", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [FIELD_ONE], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; + + const newStreamConfiguration = updatePrimaryKey(mockConfig, [FIELD_THREE], 3); + + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_THREE], + selectedFields: [], + fieldSelectionEnabled: false, + }); + }); }); }); -describe(`${addFieldToPrimaryKey.name}`, () => { +describe(`${toggleFieldInPrimaryKey.name}`, () => { it("adds a new field to the composite primary key", () => { const mockConfig: AirbyteStreamConfiguration = { ...mockStreamConfiguration, - primaryKey: [["first_pk_field"]], + primaryKey: [FIELD_ONE], }; - const newStreamConfiguration = addFieldToPrimaryKey(mockConfig, ["second_pk_field"], 3); + const newStreamConfiguration = toggleFieldInPrimaryKey(mockConfig, FIELD_TWO, 3); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - primaryKey: [["first_pk_field"], ["second_pk_field"]], - }) - ); + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_ONE, FIELD_TWO], + }); }); - it("adds the new primary key field to selectedFields when field selection is enabled", () => { - const mockConfig: AirbyteStreamConfiguration = { - ...mockStreamConfiguration, - primaryKey: [["field_one"]], - fieldSelectionEnabled: true, - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], - }; + describe("when fieldSelection is active", () => { + it("adds the new primary key field to selectedFields", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [FIELD_ONE], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; - const newStreamConfiguration = addFieldToPrimaryKey(mockConfig, ["field_three"], 5); + const newStreamConfiguration = toggleFieldInPrimaryKey(mockConfig, FIELD_THREE, 100); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - primaryKey: [["field_one"], ["field_three"]], - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }, { fieldPath: ["field_three"] }], + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_ONE, FIELD_THREE], + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }, { fieldPath: FIELD_THREE }], fieldSelectionEnabled: true, - }) - ); - }); + }); + }); - it("disables field selection when selected primary key is the last unselected field", () => { - const mockConfig: AirbyteStreamConfiguration = { - ...mockStreamConfiguration, - primaryKey: [["field_one"]], - fieldSelectionEnabled: true, - selectedFields: [{ fieldPath: ["field_one"] }, { fieldPath: ["field_two"] }], - }; + it("adds the new primary key when only one other field is unselected", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; + + const newStreamConfiguration = toggleFieldInPrimaryKey(mockConfig, FIELD_TWO, 3); + + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_TWO], + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + fieldSelectionEnabled: true, + }); + }); + + it("adds the new primary key when it is one of many unselected fields", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }], + }; + + const newStreamConfiguration = toggleFieldInPrimaryKey(mockConfig, FIELD_TWO, 100); + + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_TWO], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }); + }); + + it("disables field selection when selected primary key is the last unselected field", () => { + const mockConfig: AirbyteStreamConfiguration = { + ...mockStreamConfiguration, + primaryKey: [FIELD_ONE], + fieldSelectionEnabled: true, + selectedFields: [{ fieldPath: FIELD_ONE }, { fieldPath: FIELD_TWO }], + }; - const newStreamConfiguration = addFieldToPrimaryKey(mockConfig, ["field_three"], 3); + const newStreamConfiguration = toggleFieldInPrimaryKey(mockConfig, FIELD_THREE, 3); - expect(newStreamConfiguration).toEqual( - expect.objectContaining({ - primaryKey: [["field_one"], ["field_three"]], + expect(newStreamConfiguration).toEqual({ + primaryKey: [FIELD_ONE, FIELD_THREE], selectedFields: [], fieldSelectionEnabled: false, - }) - ); + }); + }); }); }); diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts index ac68ae5603b8..8d02b71d3ff1 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts @@ -1,6 +1,23 @@ import isEqual from "lodash/isEqual"; -import { AirbyteStreamConfiguration } from "core/request/AirbyteClient"; +import { AirbyteStreamConfiguration, SelectedFieldInfo } from "core/request/AirbyteClient"; + +/** + * Merges arrays of SelectedFieldInfo, ensuring there are no duplicates + */ +export function mergeFieldPathArrays(...args: SelectedFieldInfo[][]): SelectedFieldInfo[] { + const set = new Set(); + + args.forEach((array) => + array.forEach((selectedFieldInfo) => { + if (selectedFieldInfo.fieldPath) { + set.add(selectedFieldInfo.fieldPath); + } + }) + ); + + return Array.from(set).map((fieldPath) => ({ fieldPath })); +} /** * Updates the cursor field in AirbyteStreamConfiguration @@ -10,16 +27,19 @@ export const updateCursorField = ( selectedCursorField: string[], numberOfFieldsInStream: number ): Partial => { - const previouslySelectedFields = config?.selectedFields || []; - // If field selection is enabled, we need to be sure the new cursor is also selected - if (config?.fieldSelectionEnabled && !previouslySelectedFields.find((field) => isEqual(field, selectedCursorField))) { - if (previouslySelectedFields.length === numberOfFieldsInStream - 1) { + if (config?.fieldSelectionEnabled) { + const previouslySelectedFields = config?.selectedFields || []; + const selectedFields = mergeFieldPathArrays(previouslySelectedFields, [{ fieldPath: selectedCursorField }]); + + // If the number of selected fields is equal to the fields in the stream, field selection is disabled because all fields are selected + if (selectedFields.length === numberOfFieldsInStream) { return { cursorField: selectedCursorField, selectedFields: [], fieldSelectionEnabled: false }; } + return { fieldSelectionEnabled: true, - selectedFields: [...previouslySelectedFields, { fieldPath: selectedCursorField }], + selectedFields, cursorField: selectedCursorField, }; } @@ -27,28 +47,29 @@ export const updateCursorField = ( }; /** - * Overwrites the entire primaryKey value in AirbyteStreamConfiguration. + * Overwrites the entire primaryKey value in AirbyteStreamConfiguration, which is a composite of one or more fieldPaths */ export const updatePrimaryKey = ( config: AirbyteStreamConfiguration, compositePrimaryKey: string[][], numberOfFieldsInStream: number ): Partial => { - const previouslySelectedFields = config?.selectedFields || []; - - // If field selection is enabled, we need to be sure each fieldPath in the new primary key is also selected - if ( - config?.fieldSelectionEnabled && - !compositePrimaryKey.some((fieldPath) => previouslySelectedFields.find((field) => isEqual(field, fieldPath))) - ) { - // If the fieldPath being added to the primaryKey is the only unselected field, - // we can actually just disable field selection, since all fields are now selected. - if (previouslySelectedFields.length === numberOfFieldsInStream - 1) { + // If field selection is enabled, we need to be sure each fieldPath in the new composite primary key is also selected + if (config?.fieldSelectionEnabled) { + const previouslySelectedFields = config?.selectedFields || []; + const selectedFields = mergeFieldPathArrays( + previouslySelectedFields, + compositePrimaryKey.map((fieldPath) => ({ fieldPath })) + ); + + // If the number of selected fields is equal to the fields in the stream, field selection is disabled because all fields are selected + if (selectedFields.length === numberOfFieldsInStream) { return { primaryKey: compositePrimaryKey, selectedFields: [], fieldSelectionEnabled: false }; } + return { fieldSelectionEnabled: true, - selectedFields: [...previouslySelectedFields, ...compositePrimaryKey.map((fieldPath) => ({ fieldPath }))], + selectedFields, primaryKey: compositePrimaryKey, }; } @@ -59,15 +80,14 @@ export const updatePrimaryKey = ( }; /** - * Adds a single fieldPath to the composite primaryKey + * Toggles whether a fieldPath is part of the composite primaryKey */ -export const addFieldToPrimaryKey = ( +export const toggleFieldInPrimaryKey = ( config: AirbyteStreamConfiguration, fieldPath: string[], numberOfFieldsInStream: number ): Partial => { const fieldIsSelected = !config?.primaryKey?.find((pk) => isEqual(pk, fieldPath)); - const previouslySelectedFields = config?.selectedFields || []; let newPrimaryKey: string[][]; if (!fieldIsSelected) { @@ -77,17 +97,18 @@ export const addFieldToPrimaryKey = ( } // If field selection is enabled, we need to be sure the new fieldPath is also selected - if ( - fieldIsSelected && - config?.fieldSelectionEnabled && - !previouslySelectedFields.find((field) => isEqual(field, fieldPath)) - ) { - if (previouslySelectedFields.length === numberOfFieldsInStream - 1) { + if (fieldIsSelected && config?.fieldSelectionEnabled) { + const previouslySelectedFields = config?.selectedFields || []; + const selectedFields = mergeFieldPathArrays(previouslySelectedFields, [{ fieldPath }]); + + // If the number of selected fields is equal to the fields in the stream, field selection is disabled because all fields are selected + if (selectedFields.length === numberOfFieldsInStream) { return { primaryKey: newPrimaryKey, selectedFields: [], fieldSelectionEnabled: false }; } + return { fieldSelectionEnabled: true, - selectedFields: [...previouslySelectedFields, { fieldPath }], + selectedFields, primaryKey: newPrimaryKey, }; } From 9d6e3756ef1903e6a2cae5c4a703c040fb5f63d7 Mon Sep 17 00:00:00 2001 From: josephkmh Date: Mon, 9 Jan 2023 16:44:11 +0100 Subject: [PATCH 09/11] add comment --- .../src/components/connection/CatalogTree/StreamFieldTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx b/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx index 3f06cec5e208..2816c0b7906d 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/StreamFieldTable.tsx @@ -40,7 +40,7 @@ export const StreamFieldTable: React.FC = ({ return true; } - // path[0] is the top-level field name for all nested fields + // Nested fields cannot currently be individually deselected, so we can just check whether the top-level field has been selected return !!config?.selectedFields?.find((f) => isEqual(f.fieldPath, [field.path[0]])); }, [config] From 06f1a05235fd8ea231909acf2f1b95a79f6aadf8 Mon Sep 17 00:00:00 2001 From: josephkmh Date: Mon, 9 Jan 2023 17:09:06 +0100 Subject: [PATCH 10/11] rename method --- .../src/components/connection/CatalogTree/CatalogSection.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx index ddb73b3aaaf3..5e473ee1f2ce 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx +++ b/airbyte-webapp/src/components/connection/CatalogTree/CatalogSection.tsx @@ -119,7 +119,7 @@ const CatalogSectionInner: React.FC = ({ [config, updateStreamWithConfig, numberOfFieldsInStream] ); - const onSelectedFieldsUpdate = (fieldPath: string[], isSelected: boolean) => { + const onToggleFieldSelected = (fieldPath: string[], isSelected: boolean) => { const previouslySelectedFields = config?.selectedFields || []; if (!config?.fieldSelectionEnabled && !isSelected) { @@ -245,7 +245,7 @@ const CatalogSectionInner: React.FC = ({ syncSchemaFields={flattenedFields} onCursorSelect={onCursorSelect} onPkSelect={onPkSelect} - handleFieldToggle={onSelectedFieldsUpdate} + handleFieldToggle={onToggleFieldSelected} primaryKeyIndexerType={pkType} cursorIndexerType={cursorType} shouldDefinePrimaryKey={shouldDefinePk} From d1938199b88db9560ba449607003442468963cd2 Mon Sep 17 00:00:00 2001 From: josephkmh Date: Tue, 10 Jan 2023 10:58:26 +0100 Subject: [PATCH 11/11] fix merging duplicates of complex fieldPaths --- .../streamConfigHelpers.test.ts | 15 +++++++++++++++ .../streamConfigHelpers/streamConfigHelpers.ts | 7 ++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts index 75e4855daffa..8e5065e42e7f 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.test.ts @@ -30,6 +30,21 @@ describe(`${mergeFieldPathArrays.name}`, () => { { fieldPath: FIELD_THREE }, ]); }); + + it("merges two arrays of complex fieldPaths without duplicates", () => { + const arr1 = [{ fieldPath: [...FIELD_ONE, ...FIELD_TWO] }, { fieldPath: [...FIELD_TWO, ...FIELD_THREE] }]; + const arr2 = [ + { fieldPath: [...FIELD_ONE, ...FIELD_TWO] }, + { fieldPath: [...FIELD_TWO, ...FIELD_THREE] }, + { fieldPath: [...FIELD_ONE, ...FIELD_THREE] }, + ]; + + expect(mergeFieldPathArrays(arr1, arr2)).toEqual([ + { fieldPath: [...FIELD_ONE, ...FIELD_TWO] }, + { fieldPath: [...FIELD_TWO, ...FIELD_THREE] }, + { fieldPath: [...FIELD_ONE, ...FIELD_THREE] }, + ]); + }); }); describe(`${updateCursorField.name}`, () => { diff --git a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts index 8d02b71d3ff1..e3dacd8ebcef 100644 --- a/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts +++ b/airbyte-webapp/src/components/connection/CatalogTree/streamConfigHelpers/streamConfigHelpers.ts @@ -6,17 +6,18 @@ import { AirbyteStreamConfiguration, SelectedFieldInfo } from "core/request/Airb * Merges arrays of SelectedFieldInfo, ensuring there are no duplicates */ export function mergeFieldPathArrays(...args: SelectedFieldInfo[][]): SelectedFieldInfo[] { - const set = new Set(); + const set = new Set(); args.forEach((array) => array.forEach((selectedFieldInfo) => { if (selectedFieldInfo.fieldPath) { - set.add(selectedFieldInfo.fieldPath); + const key = JSON.stringify(selectedFieldInfo.fieldPath); + set.add(key); } }) ); - return Array.from(set).map((fieldPath) => ({ fieldPath })); + return Array.from(set).map((key) => ({ fieldPath: JSON.parse(key) })); } /**