-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🪟 🎉 Add column selection UI to new stream table #21058
Merged
Merged
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
cc625d2
add column selection UI to new stream table
josephkmh b79bb18
disable experiment by default
josephkmh a6c6b87
add option to toggle all selected fields
josephkmh 94f331f
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh 81a0330
fix header styling
josephkmh 972be8f
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh 84db0c0
fix missing dependencies
josephkmh 625bb89
fix checkbox warnings
josephkmh 9c6e31a
add ability to select/deselect all, ignoring pk and cursor
josephkmh e9d8a22
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh e6914ec
typo in merge
josephkmh 59d0267
refactor method for single field toggle, add tests
josephkmh b123ed3
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh c0278a4
test that cursor & pk are selected when toggling all fields
josephkmh 4b82714
support source defined pk & cursor
josephkmh 25ebac7
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh 706163d
disable field selection checkboxes in readonly mode
josephkmh d311cb9
omit selectedFields if field selection disabled
josephkmh 05e56af
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh 427a3b6
disable deselection of parent field when child is part of pk or is cu…
josephkmh cf5e863
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh 16d0b57
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh 22f08b4
fix disabling checkboxes in readonly mode
josephkmh b5769c8
missing dependency
josephkmh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,4 @@ | ||
import { FormikErrors, getIn } from "formik"; | ||
import isEqual from "lodash/isEqual"; | ||
import React, { memo, useCallback, useMemo } from "react"; | ||
import { useToggle } from "react-use"; | ||
|
||
|
@@ -13,7 +12,6 @@ import { | |
DestinationSyncMode, | ||
NamespaceDefinitionType, | ||
SyncMode, | ||
SelectedFieldInfo, | ||
} from "core/request/AirbyteClient"; | ||
import { useDestinationNamespace } from "hooks/connection/useDestinationNamespace"; | ||
import { useNewTableDesignExperiment } from "hooks/connection/useNewTableDesignExperiment"; | ||
|
@@ -27,6 +25,8 @@ import { | |
updatePrimaryKey, | ||
toggleFieldInPrimaryKey, | ||
updateCursorField, | ||
updateFieldSelected, | ||
toggleAllFieldsSelected, | ||
} from "./streamConfigHelpers/streamConfigHelpers"; | ||
import { StreamFieldTable } from "./StreamFieldTable"; | ||
import { StreamHeader } from "./StreamHeader"; | ||
|
@@ -51,8 +51,14 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({ | |
errors, | ||
changedSelected, | ||
}) => { | ||
const { stream, config } = streamNode; | ||
const isNewTableDesignEnabled = useNewTableDesignExperiment(); | ||
|
||
const fields = useMemo(() => { | ||
const traversedFields = traverseSchemaToField(stream?.jsonSchema, stream?.name); | ||
return traversedFields.sort(naturalComparatorBy((field) => field.cleanedName)); | ||
}, [stream?.jsonSchema, stream?.name]); | ||
|
||
const numberOfFieldsInStream = Object.keys(streamNode?.stream?.jsonSchema?.properties).length ?? 0; | ||
|
||
const { | ||
|
@@ -61,7 +67,6 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({ | |
const { mode } = useConnectionFormService(); | ||
|
||
const [isRowExpanded, onExpand] = useToggle(false); | ||
const { stream, config } = streamNode; | ||
|
||
const updateStreamWithConfig = useCallback( | ||
(config: Partial<AirbyteStreamConfiguration>) => updateStream(streamNode.id, config), | ||
|
@@ -86,9 +91,7 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({ | |
if (!config) { | ||
return; | ||
} | ||
|
||
const updatedConfig = toggleFieldInPrimaryKey(config, pkPath, numberOfFieldsInStream); | ||
|
||
updateStreamWithConfig(updatedConfig); | ||
}, | ||
[config, updateStreamWithConfig, numberOfFieldsInStream] | ||
|
@@ -99,9 +102,7 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({ | |
if (!config) { | ||
return; | ||
} | ||
|
||
const updatedConfig = updateCursorField(config, cursorField, numberOfFieldsInStream); | ||
|
||
updateStreamWithConfig(updatedConfig); | ||
}, | ||
[config, numberOfFieldsInStream, updateStreamWithConfig] | ||
|
@@ -112,43 +113,30 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({ | |
if (!config) { | ||
return; | ||
} | ||
|
||
const updatedConfig = updatePrimaryKey(config, newPrimaryKey, numberOfFieldsInStream); | ||
|
||
updateStreamWithConfig(updatedConfig); | ||
}, | ||
[config, updateStreamWithConfig, numberOfFieldsInStream] | ||
); | ||
|
||
const onToggleFieldSelected = (fieldPath: string[], isSelected: boolean) => { | ||
const previouslySelectedFields = config?.selectedFields || []; | ||
|
||
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 onToggleAllFieldsSelected = useCallback(() => { | ||
if (!config) { | ||
return; | ||
} | ||
}; | ||
const updatedConfig = toggleAllFieldsSelected(config); | ||
updateStreamWithConfig(updatedConfig); | ||
}, [config, updateStreamWithConfig]); | ||
|
||
const onToggleFieldSelected = useCallback( | ||
(fieldPath: string[], isSelected: boolean) => { | ||
if (!config) { | ||
return; | ||
} | ||
const updatedConfig = updateFieldSelected({ config, fields, fieldPath, isSelected, numberOfFieldsInStream }); | ||
updateStreamWithConfig(updatedConfig); | ||
}, | ||
[config, fields, numberOfFieldsInStream, updateStreamWithConfig] | ||
); | ||
Comment on lines
+130
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been extracted to |
||
|
||
const pkRequired = config?.destinationSyncMode === DestinationSyncMode.append_dedup; | ||
const cursorRequired = config?.syncMode === SyncMode.incremental; | ||
|
@@ -172,12 +160,6 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({ | |
namespaceFormat, | ||
}) ?? ""; | ||
|
||
const fields = useMemo(() => { | ||
const traversedFields = traverseSchemaToField(stream?.jsonSchema, stream?.name); | ||
|
||
return traversedFields.sort(naturalComparatorBy((field) => field.cleanedName)); | ||
}, [stream?.jsonSchema, stream?.name]); | ||
|
||
const flattenedFields = useMemo(() => flatten(fields), [fields]); | ||
|
||
const primitiveFields = useMemo<SyncSchemaField[]>( | ||
|
@@ -231,11 +213,13 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({ | |
onCursorSelect={onCursorSelect} | ||
onPkSelect={onPkSelect} | ||
onSelectedChange={onSelectStream} | ||
handleFieldToggle={onToggleFieldSelected} | ||
shouldDefinePk={shouldDefinePk} | ||
shouldDefineCursor={shouldDefineCursor} | ||
isCursorDefinitionSupported={cursorRequired} | ||
isPKDefinitionSupported={pkRequired} | ||
stream={stream} | ||
toggleAllFieldsSelected={onToggleAllFieldsSelected} | ||
/> | ||
) : ( | ||
<div className={styles.streamFieldTableContainer}> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
126 changes: 126 additions & 0 deletions
126
...nnection/CatalogTree/next/StreamDetailsPanel/StreamFieldsTable/StreamFieldsTable.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
import { mockStreamConfiguration } from "test-utils/mock-data/mockAirbyteStreamConfiguration"; | ||
|
||
import { AirbyteStreamConfiguration } from "core/request/AirbyteClient"; | ||
|
||
import { isChildFieldCursor, isChildFieldPrimaryKey, isCursor, isPrimaryKey } from "./StreamFieldsTable"; | ||
|
||
const mockIncrementalConfig: AirbyteStreamConfiguration = { | ||
...mockStreamConfiguration, | ||
syncMode: "incremental", | ||
}; | ||
|
||
const mockIncrementalDedupConfig: AirbyteStreamConfiguration = { | ||
...mockStreamConfiguration, | ||
syncMode: "incremental", | ||
destinationSyncMode: "append_dedup", | ||
}; | ||
|
||
describe(`${isCursor.name}`, () => { | ||
it("returns true if the path matches the cursor", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalConfig, | ||
cursorField: ["my_cursor"], | ||
}; | ||
expect(isCursor(config, ["my_cursor"])).toBe(true); | ||
}); | ||
|
||
it("returns false if there is no cursor", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalConfig, | ||
cursorField: undefined, | ||
}; | ||
expect(isCursor(config, ["my_cursor"])).toBe(false); | ||
}); | ||
|
||
it("returns false if the path does not match the cursor", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalConfig, | ||
cursorField: ["my_cursor"], | ||
}; | ||
expect(isCursor(config, ["some_other_field"])).toBe(false); | ||
}); | ||
}); | ||
|
||
describe(`${isChildFieldCursor.name}`, () => { | ||
it("returns true if the path is the parent of the cursor", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalConfig, | ||
cursorField: ["my_cursor", "nested_field"], | ||
}; | ||
expect(isChildFieldCursor(config, ["my_cursor"])).toBe(true); | ||
}); | ||
|
||
it("returns false if there is no cursor", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalConfig, | ||
cursorField: undefined, | ||
}; | ||
expect(isChildFieldCursor(config, ["my_cursor"])).toBe(false); | ||
}); | ||
|
||
it("returns false if the path does not match the cursor", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalConfig, | ||
cursorField: ["my_cursor", "nested_field"], | ||
}; | ||
expect(isChildFieldCursor(config, ["some_other_field"])).toBe(false); | ||
}); | ||
}); | ||
|
||
describe(`${isPrimaryKey.name}`, () => { | ||
it("returns true if the path matches any part of the primary key", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalDedupConfig, | ||
primaryKey: [["some_other_pk"], ["my_pk"]], | ||
}; | ||
expect(isPrimaryKey(config, ["my_pk"])).toBe(true); | ||
}); | ||
|
||
it("returns false if there is no primary key", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalDedupConfig, | ||
primaryKey: undefined, | ||
}; | ||
expect(isPrimaryKey(config, ["my_pk"])).toBe(false); | ||
}); | ||
|
||
it("returns false if the path does not match any part of the primary key", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalDedupConfig, | ||
primaryKey: [["some_other_pk"], ["my_pk"]], | ||
}; | ||
expect(isPrimaryKey(config, ["unrelated_field"])).toBe(false); | ||
}); | ||
}); | ||
|
||
describe(`${isChildFieldPrimaryKey.name}`, () => { | ||
it("returns true if the path is the parent of any part of the primary key", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalDedupConfig, | ||
primaryKey: [ | ||
["some_other_pk", "nested_field"], | ||
["my_pk", "nested_field"], | ||
], | ||
}; | ||
expect(isChildFieldPrimaryKey(config, ["my_pk"])).toBe(true); | ||
}); | ||
|
||
it("returns false if there is no primary key", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalDedupConfig, | ||
primaryKey: undefined, | ||
}; | ||
expect(isChildFieldPrimaryKey(config, ["my_pk"])).toBe(false); | ||
}); | ||
|
||
it("returns false if the path is not the parent of any part of the primary key", () => { | ||
const config: AirbyteStreamConfiguration = { | ||
...mockIncrementalDedupConfig, | ||
primaryKey: [ | ||
["some_other_pk", "nested_field"], | ||
["my_pk", "nested_field"], | ||
], | ||
}; | ||
expect(isChildFieldPrimaryKey(config, ["unrelated_field"])).toBe(false); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has just been moved up in the file, not changed!