Skip to content
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 24 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from all 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 Jan 5, 2023
b79bb18
disable experiment by default
josephkmh Jan 5, 2023
a6c6b87
add option to toggle all selected fields
josephkmh Jan 5, 2023
94f331f
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh Jan 5, 2023
81a0330
fix header styling
josephkmh Jan 5, 2023
972be8f
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh Jan 5, 2023
84db0c0
fix missing dependencies
josephkmh Jan 5, 2023
625bb89
fix checkbox warnings
josephkmh Jan 5, 2023
9c6e31a
add ability to select/deselect all, ignoring pk and cursor
josephkmh Jan 5, 2023
e9d8a22
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh Jan 11, 2023
e6914ec
typo in merge
josephkmh Jan 11, 2023
59d0267
refactor method for single field toggle, add tests
josephkmh Jan 12, 2023
b123ed3
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh Jan 12, 2023
c0278a4
test that cursor & pk are selected when toggling all fields
josephkmh Jan 12, 2023
4b82714
support source defined pk & cursor
josephkmh Jan 12, 2023
25ebac7
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh Jan 12, 2023
706163d
disable field selection checkboxes in readonly mode
josephkmh Jan 16, 2023
d311cb9
omit selectedFields if field selection disabled
josephkmh Jan 16, 2023
05e56af
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh Jan 19, 2023
427a3b6
disable deselection of parent field when child is part of pk or is cu…
josephkmh Jan 19, 2023
cf5e863
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh Jan 23, 2023
16d0b57
Merge branch 'master' into joey/column-selection-new-stream-table
josephkmh Jan 24, 2023
22f08b4
fix disabling checkboxes in readonly mode
josephkmh Jan 24, 2023
b5769c8
missing dependency
josephkmh Jan 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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";

Expand All @@ -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";
Expand All @@ -27,6 +25,8 @@ import {
updatePrimaryKey,
toggleFieldInPrimaryKey,
updateCursorField,
updateFieldSelected,
toggleAllFieldsSelected,
} from "./streamConfigHelpers/streamConfigHelpers";
import { StreamFieldTable } from "./StreamFieldTable";
import { StreamHeader } from "./StreamHeader";
Expand All @@ -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]);

Comment on lines +57 to +61
Copy link
Contributor Author

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!

const numberOfFieldsInStream = Object.keys(streamNode?.stream?.jsonSchema?.properties).length ?? 0;

const {
Expand All @@ -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),
Expand All @@ -86,9 +91,7 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({
if (!config) {
return;
}

const updatedConfig = toggleFieldInPrimaryKey(config, pkPath, numberOfFieldsInStream);

updateStreamWithConfig(updatedConfig);
},
[config, updateStreamWithConfig, numberOfFieldsInStream]
Expand All @@ -99,9 +102,7 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({
if (!config) {
return;
}

const updatedConfig = updateCursorField(config, cursorField, numberOfFieldsInStream);

updateStreamWithConfig(updatedConfig);
},
[config, numberOfFieldsInStream, updateStreamWithConfig]
Expand All @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been extracted to streamConfigHelpers to make unit testing easier (see streamConfigHelpers.test.ts)


const pkRequired = config?.destinationSyncMode === DestinationSyncMode.append_dedup;
const cursorRequired = config?.syncMode === SyncMode.incremental;
Expand All @@ -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[]>(
Expand Down Expand Up @@ -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}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ export const CatalogTreeBody: React.FC<CatalogTreeBodyProps> = ({ streams, chang
if (streamNode) {
const newStreamNode = setIn(streamNode, "config", { ...streamNode.config, ...newConfig });

// config.selectedFields must be removed if fieldSelection is disabled
if (!newStreamNode.config.fieldSelectionEnabled) {
delete newStreamNode.config.selectedFields;
}

onStreamChanged(newStreamNode);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@ interface StreamDetailsPanelProps extends StreamFieldsTableProps {
onClose: () => void;
onSelectedChange: () => void;
stream?: AirbyteStream;
handleFieldToggle: (fieldPath: string[], isSelected: boolean) => void;
toggleAllFieldsSelected: () => void;
}

export const StreamDetailsPanel: React.FC<StreamDetailsPanelProps> = ({
stream,
config,
disabled,
handleFieldToggle,
onPkSelect,
onCursorSelect,
onClose,
Expand All @@ -29,6 +32,7 @@ export const StreamDetailsPanel: React.FC<StreamDetailsPanelProps> = ({
isCursorDefinitionSupported,
isPKDefinitionSupported,
syncSchemaFields,
toggleAllFieldsSelected,
}) => {
return (
<Dialog className={styles.dialog} open onClose={onClose}>
Expand All @@ -45,12 +49,14 @@ export const StreamDetailsPanel: React.FC<StreamDetailsPanelProps> = ({
<StreamFieldsTable
config={config}
syncSchemaFields={syncSchemaFields}
handleFieldToggle={handleFieldToggle}
onCursorSelect={onCursorSelect}
onPkSelect={onPkSelect}
shouldDefinePk={shouldDefinePk}
shouldDefineCursor={shouldDefineCursor}
isCursorDefinitionSupported={isCursorDefinitionSupported}
isPKDefinitionSupported={isPKDefinitionSupported}
toggleAllFieldsSelected={toggleAllFieldsSelected}
/>
</div>
</Dialog.Panel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ $cell-left-padding: variables.$spacing-xl + variables.$spacing-sm;
line-height: 15px;
text-transform: capitalize;

&--syncCell {
width: 0; /** Collapses the th to the exact size of its content **/
padding-right: variables.$spacing-lg;
}

&:first-child {
padding-left: $cell-left-padding;
border-radius: 0;
Expand Down
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);
});
});
Loading