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

🪟 🔧 Move auto-detect schema changes feature flag to FeatureService #20232

Merged
merged 3 commits into from
Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -5,7 +5,6 @@ import { useNavigate } from "react-router-dom";
import LoadingSchema from "components/LoadingSchema";

import { DestinationRead, SourceRead } from "core/request/AirbyteClient";
import { useIsAutoDetectSchemaChangesEnabled } from "hooks/connection/useIsAutoDetectSchemaChangesEnabled";
import {
ConnectionFormServiceProvider,
tidyConnectionFormValues,
Expand Down Expand Up @@ -43,7 +42,7 @@ const CreateConnectionFormInner: React.FC<CreateConnectionPropsInner> = ({ schem
const navigate = useNavigate();
const canEditDataGeographies = useFeature(FeatureItem.AllowChangeDataGeographies);
const { mutateAsync: createConnection } = useCreateConnection();
const isAutoDetectSchemaChangesEnabled = useIsAutoDetectSchemaChangesEnabled();
const allowAutoDetectSchemaChanges = useFeature(FeatureItem.AllowAutoDetectSchemaChanges);
const { clearFormChange } = useFormChangeTrackerService();

const workspaceId = useCurrentWorkspaceId();
Expand All @@ -59,7 +58,7 @@ const CreateConnectionFormInner: React.FC<CreateConnectionPropsInner> = ({ schem
workspaceId,
mode,
allowSubOneHourCronExpressions,
isAutoDetectSchemaChangesEnabled
allowAutoDetectSchemaChanges
);

try {
Expand Down Expand Up @@ -94,7 +93,7 @@ const CreateConnectionFormInner: React.FC<CreateConnectionPropsInner> = ({ schem
workspaceId,
mode,
allowSubOneHourCronExpressions,
isAutoDetectSchemaChangesEnabled,
allowAutoDetectSchemaChanges,
createConnection,
connection.source,
connection.destination,
Expand All @@ -119,7 +118,7 @@ const CreateConnectionFormInner: React.FC<CreateConnectionPropsInner> = ({ schem
validationSchema={createConnectionValidationSchema({
mode,
allowSubOneHourCronExpressions,
isAutoDetectSchemaChangesEnabled,
allowAutoDetectSchemaChanges,
})}
onSubmit={onFormSubmit}
>
Expand Down
7 changes: 3 additions & 4 deletions airbyte-webapp/src/components/EntityTable/ConnectionTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { CellProps } from "react-table";
import { Table, SortableTableHeader } from "components/ui/Table";

import { ConnectionScheduleType, SchemaChange } from "core/request/AirbyteClient";
import { useIsAutoDetectSchemaChangesEnabled } from "hooks/connection/useIsAutoDetectSchemaChangesEnabled";
import { FeatureItem, useFeature } from "hooks/services/Feature";
import { useQuery } from "hooks/useQuery";

Expand All @@ -29,7 +28,7 @@ interface IProps {
const ConnectionTable: React.FC<IProps> = ({ data, entity, onClickRow, onSync }) => {
const navigate = useNavigate();
const query = useQuery<{ sortBy?: string; order?: SortOrderEnum }>();
const isSchemaChangesEnabled = useIsAutoDetectSchemaChangesEnabled();
const allowAutoDetectSchemaChanges = useFeature(FeatureItem.AllowAutoDetectSchemaChanges);
const allowSync = useFeature(FeatureItem.AllowSync);

const sortBy = query.sortBy || "entityName";
Expand Down Expand Up @@ -164,7 +163,7 @@ const ConnectionTable: React.FC<IProps> = ({ data, entity, onClickRow, onSync })
isSyncing={row.original.isSyncing}
isManual={row.original.scheduleType === ConnectionScheduleType.manual}
onSync={onSync}
hasBreakingChange={isSchemaChangesEnabled && row.original.schemaChange === SchemaChange.breaking}
hasBreakingChange={allowAutoDetectSchemaChanges && row.original.schemaChange === SchemaChange.breaking}
allowSync={allowSync}
/>
),
Expand All @@ -176,7 +175,7 @@ const ConnectionTable: React.FC<IProps> = ({ data, entity, onClickRow, onSync })
Cell: ({ cell }: CellProps<ITableDataItem>) => <ConnectionSettingsCell id={cell.value} />,
},
],
[sortBy, sortOrder, entity, onSortClick, onSync, allowSync, isSchemaChangesEnabled]
[sortBy, sortOrder, entity, onSortClick, onSync, allowSync, allowAutoDetectSchemaChanges]
);

return <Table columns={columns} data={sortingData} onClickRow={onClickRow} erroredRows />;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from "react";

import { SchemaChange } from "core/request/AirbyteClient";
import { useIsAutoDetectSchemaChangesEnabled } from "hooks/connection/useIsAutoDetectSchemaChangesEnabled";
import { FeatureItem, useFeature } from "hooks/services/Feature";

import { ChangesStatusIcon } from "./ChangesStatusIcon";
import styles from "./StatusCell.module.scss";
Expand All @@ -28,7 +28,7 @@ export const StatusCell: React.FC<StatusCellProps> = ({
schemaChange,
hasBreakingChange,
}) => {
const isSchemaChangesEnabled = useIsAutoDetectSchemaChangesEnabled();
const allowAutoDetectSchemaChanges = useFeature(FeatureItem.AllowAutoDetectSchemaChanges);

return (
<div className={styles.container}>
Expand All @@ -41,7 +41,7 @@ export const StatusCell: React.FC<StatusCellProps> = ({
hasBreakingChange={hasBreakingChange}
allowSync={allowSync}
/>
{isSchemaChangesEnabled && <ChangesStatusIcon schemaChange={schemaChange} />}
{allowAutoDetectSchemaChanges && <ChangesStatusIcon schemaChange={schemaChange} />}
</div>
);
};

This file was deleted.

9 changes: 4 additions & 5 deletions airbyte-webapp/src/hooks/connection/useSchemaChanges.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { useMemo } from "react";

import { SchemaChange } from "core/request/AirbyteClient";

import { useIsAutoDetectSchemaChangesEnabled } from "./useIsAutoDetectSchemaChangesEnabled";
import { FeatureItem, useFeature } from "hooks/services/Feature";

export const useSchemaChanges = (schemaChange: SchemaChange) => {
const isSchemaChangesEnabled = useIsAutoDetectSchemaChangesEnabled();
const allowAutoDetectSchemaChanges = useFeature(FeatureItem.AllowAutoDetectSchemaChanges);

return useMemo(() => {
const hasSchemaChanges = isSchemaChangesEnabled && schemaChange !== SchemaChange.no_change;
const hasSchemaChanges = allowAutoDetectSchemaChanges && schemaChange !== SchemaChange.no_change;
const hasBreakingSchemaChange = hasSchemaChanges && schemaChange === SchemaChange.breaking;
const hasNonBreakingSchemaChange = hasSchemaChanges && schemaChange === SchemaChange.non_breaking;

Expand All @@ -18,5 +17,5 @@ export const useSchemaChanges = (schemaChange: SchemaChange) => {
hasBreakingSchemaChange,
hasNonBreakingSchemaChange,
};
}, [isSchemaChangesEnabled, schemaChange]);
}, [allowAutoDetectSchemaChanges, schemaChange]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ export const tidyConnectionFormValues = (
workspaceId: string,
mode: ConnectionFormMode,
allowSubOneHourCronExpressions: boolean,
isAutoDetectSchemaChangesEnabled: boolean,
allowAutoDetectSchemaChanges: boolean,
operations?: OperationRead[]
): ValuesProps => {
// TODO (https://github.com/airbytehq/airbyte/issues/17279): We should try to fix the types so we don't need the casting.
const formValues: ConnectionFormValues = createConnectionValidationSchema({
mode,
allowSubOneHourCronExpressions,
isAutoDetectSchemaChangesEnabled,
allowAutoDetectSchemaChanges,
}).cast(values, {
context: { isRequest: true },
}) as unknown as ConnectionFormValues;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ export interface Experiments {
"authPage.oauth.position": "top" | "bottom";
"connection.onboarding.sources": string;
"connection.onboarding.destinations": string;
"connection.autoDetectSchemaChanges": boolean;
}
1 change: 1 addition & 0 deletions airbyte-webapp/src/hooks/services/Feature/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

export enum FeatureItem {
AllowAutoDetectSchemaChanges = "ALLOW_AUTO_DETECT_SCHEMA_CHANGES",
AllowUploadCustomImage = "ALLOW_UPLOAD_CUSTOM_IMAGE",
AllowCustomDBT = "ALLOW_CUSTOM_DBT",
AllowDBTCloudIntegration = "ALLOW_DBT_CLOUD_INTEGRATION",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { Action, Namespace } from "core/analytics";
import { getFrequencyFromScheduleData } from "core/analytics/utils";
import { toWebBackendConnectionUpdate } from "core/domain/connection";
import { useConfirmCatalogDiff } from "hooks/connection/useConfirmCatalogDiff";
import { useIsAutoDetectSchemaChangesEnabled } from "hooks/connection/useIsAutoDetectSchemaChangesEnabled";
import { PageTrackingCodes, useAnalyticsService, useTrackPage } from "hooks/services/Analytics";
import { useConnectionEditService } from "hooks/services/ConnectionEdit/ConnectionEditService";
import {
Expand All @@ -33,7 +32,7 @@ import styles from "./ConnectionReplicationTab.module.scss";
import { ResetWarningModal } from "./ResetWarningModal";

export const ConnectionReplicationTab: React.FC = () => {
const isAutoDetectSchemaChangesEnabled = useIsAutoDetectSchemaChangesEnabled();
const allowAutoDetectSchemaChanges = useFeature(FeatureItem.AllowAutoDetectSchemaChanges);
const analyticsService = useAnalyticsService();
const connectionService = useConnectionService();
const workspaceId = useCurrentWorkspaceId();
Expand Down Expand Up @@ -86,7 +85,7 @@ export const ConnectionReplicationTab: React.FC = () => {
workspaceId,
mode,
allowSubOneHourCronExpressions,
isAutoDetectSchemaChangesEnabled,
allowAutoDetectSchemaChanges,
connection.operations
);

Expand Down Expand Up @@ -145,7 +144,7 @@ export const ConnectionReplicationTab: React.FC = () => {
workspaceId,
mode,
allowSubOneHourCronExpressions,
isAutoDetectSchemaChangesEnabled,
allowAutoDetectSchemaChanges,
connection.operations,
connection.catalogDiff?.transforms,
connection.syncCatalog.streams,
Expand Down Expand Up @@ -175,7 +174,7 @@ export const ConnectionReplicationTab: React.FC = () => {
validationSchema={createConnectionValidationSchema({
mode,
allowSubOneHourCronExpressions,
isAutoDetectSchemaChangesEnabled,
allowAutoDetectSchemaChanges,
})}
onSubmit={onFormSubmit}
enableReinitialize
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import userEvent from "@testing-library/user-event";
import { mockConnection, TestWrapper } from "test-utils/testutils";

import { SchemaChange } from "core/request/AirbyteClient";
import { FeatureItem } from "hooks/services/Feature";
import en from "locales/en.json";

// eslint-disable-next-line css-modules/no-unused-class
Expand All @@ -20,9 +21,11 @@ jest.doMock("views/Connection/ConnectionForm/components/refreshSourceSchemaWithC
useRefreshSourceSchemaWithConfirmationOnDirty: mockUseRefreshSourceSchemaWithConfirmationOnDirty,
}));

jest.mock("hooks/connection/useIsAutoDetectSchemaChangesEnabled", () => ({
useIsAutoDetectSchemaChangesEnabled: () => true,
}));
const TestWrapperWithAutoDetectSchema: React.FC<React.PropsWithChildren<Record<string, unknown>>> = ({ children }) => (
<TestWrapper features={[FeatureItem.AllowAutoDetectSchemaChanges]}>{children}</TestWrapper>
);

const renderComponent = () => render(<SchemaChangesDetected />, { wrapper: TestWrapperWithAutoDetectSchema });

// eslint-disable-next-line @typescript-eslint/no-var-requires
const { SchemaChangesDetected } = require("./SchemaChangesDetected");
Expand All @@ -43,7 +46,7 @@ describe("<SchemaChangesDetected />", () => {
schemaRefreshing: false,
});

const { queryByTestId } = render(<SchemaChangesDetected />, { wrapper: TestWrapper });
const { queryByTestId } = renderComponent();

expect(queryByTestId("schemaChagnesDetected")).toBeFalsy();
});
Expand All @@ -55,7 +58,7 @@ describe("<SchemaChangesDetected />", () => {
schemaRefreshing: false,
});

const { getByTestId } = render(<SchemaChangesDetected />, { wrapper: TestWrapper });
const { getByTestId } = renderComponent();

expect(getByTestId("schemaChangesDetected")).toHaveClass(styles.breaking);
expect(getByTestId("schemaChangesDetected")).not.toHaveClass(styles.nonBreaking);
Expand All @@ -69,7 +72,7 @@ describe("<SchemaChangesDetected />", () => {
schemaRefreshing: false,
});

const { getByTestId } = render(<SchemaChangesDetected />, { wrapper: TestWrapper });
const { getByTestId } = renderComponent();

expect(getByTestId("schemaChangesDetected")).toHaveClass(styles.nonBreaking);
expect(getByTestId("schemaChangesDetected")).not.toHaveClass(styles.breaking);
Expand All @@ -86,7 +89,7 @@ describe("<SchemaChangesDetected />", () => {
const refreshSpy = jest.fn();
mockUseRefreshSourceSchemaWithConfirmationOnDirty.mockReturnValue(refreshSpy);

const { getByRole } = render(<SchemaChangesDetected />, { wrapper: TestWrapper });
const { getByRole } = renderComponent();

userEvent.click(getByRole("button"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { mockSourceDefinition } from "test-utils/mock-data/mockSourceDefinition"
import { mockConnection, TestWrapper } from "test-utils/testutils";

import { ConnectionStatus, SchemaChange } from "core/request/AirbyteClient";
import { defaultFeatures, FeatureItem } from "hooks/services/Feature";

// eslint-disable-next-line css-modules/no-unused-class
import styles from "./StatusMainInfo.module.scss";
Expand All @@ -26,9 +27,9 @@ jest.doMock("views/Connection/ConnectionForm/components/refreshSourceSchemaWithC
useRefreshSourceSchemaWithConfirmationOnDirty: jest.fn(),
}));

jest.mock("hooks/connection/useIsAutoDetectSchemaChangesEnabled", () => ({
useIsAutoDetectSchemaChangesEnabled: () => true,
}));
const TestWrapperWithAutoDetectSchema: React.FC<React.PropsWithChildren<Record<string, unknown>>> = ({ children }) => (
<TestWrapper features={[...defaultFeatures, FeatureItem.AllowAutoDetectSchemaChanges]}>{children}</TestWrapper>
);

// eslint-disable-next-line @typescript-eslint/no-var-requires
const { StatusMainInfo } = require("./StatusMainInfo");
Expand All @@ -42,7 +43,7 @@ describe("<StatusMainInfo />", () => {
});

it("renders", () => {
const { getByTestId, queryByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapper });
const { getByTestId, queryByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapperWithAutoDetectSchema });

expect(getByTestId("statusMainInfo")).toBeDefined();

Expand All @@ -61,7 +62,7 @@ describe("<StatusMainInfo />", () => {
schemaHasBeenRefreshed: false,
});

const { getByTestId, queryByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapper });
const { getByTestId, queryByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapperWithAutoDetectSchema });

expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.breaking);
expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.nonBreaking);
Expand All @@ -76,7 +77,7 @@ describe("<StatusMainInfo />", () => {
schemaHasBeenRefreshed: false,
});

const { getByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapper });
const { getByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapperWithAutoDetectSchema });

expect(getByTestId("statusMainInfo-sourceLink")).toHaveClass(styles.breaking);
expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.nonBreaking);
Expand All @@ -91,7 +92,7 @@ describe("<StatusMainInfo />", () => {
schemaHasBeenRefreshed: false,
});

const { getByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapper });
const { getByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapperWithAutoDetectSchema });

expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.breaking);
expect(getByTestId("statusMainInfo-sourceLink")).toHaveClass(styles.nonBreaking);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import { Heading } from "components/ui/Heading";
import { Input } from "components/ui/Input";

import { NamespaceDefinitionType } from "core/request/AirbyteClient";
import { useIsAutoDetectSchemaChangesEnabled } from "hooks/connection/useIsAutoDetectSchemaChangesEnabled";
import { useConnectionFormService } from "hooks/services/ConnectionForm/ConnectionFormService";
import { FeatureItem, useFeature } from "hooks/services/Feature";
import { useFormChangeTrackerService } from "hooks/services/FormChangeTracker";
import { ValuesProps } from "hooks/services/useConnectionHook";

Expand All @@ -34,7 +34,7 @@ interface ConnectionFormFieldsProps {
}

export const ConnectionFormFields: React.FC<ConnectionFormFieldsProps> = ({ values, isSubmitting, dirty }) => {
const isSchemaChangesEnabled = useIsAutoDetectSchemaChangesEnabled();
const allowAutoDetectSchemaChanges = useFeature(FeatureItem.AllowAutoDetectSchemaChanges);

const { mode, formId } = useConnectionFormService();
const { formatMessage } = useIntl();
Expand All @@ -59,7 +59,7 @@ export const ConnectionFormFields: React.FC<ConnectionFormFieldsProps> = ({ valu
<div className={styles.formContainer}>
<Section title={<FormattedMessage id="connection.transfer" />}>
<ScheduleField />
{isSchemaChangesEnabled && (
{allowAutoDetectSchemaChanges && (
<Field name="nonBreakingChangesPreference" component={NonBreakingChangesPreferenceField} />
)}
</Section>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,13 @@ export function useDefaultTransformation(): OperationCreate {
interface CreateConnectionValidationSchemaArgs {
allowSubOneHourCronExpressions: boolean;
mode: ConnectionFormMode;
isAutoDetectSchemaChangesEnabled: boolean;
allowAutoDetectSchemaChanges: boolean;
}

export const createConnectionValidationSchema = ({
mode,
allowSubOneHourCronExpressions,
isAutoDetectSchemaChangesEnabled,
allowAutoDetectSchemaChanges,
}: CreateConnectionValidationSchemaArgs) =>
yup
.object({
Expand Down Expand Up @@ -130,7 +130,7 @@ export const createConnectionValidationSchema = ({
.defined("form.empty.error"),
});
}),
nonBreakingChangesPreference: isAutoDetectSchemaChangesEnabled
nonBreakingChangesPreference: allowAutoDetectSchemaChanges
? yup.mixed().oneOf(Object.values(NonBreakingChangesPreference)).required("form.empty.error")
: yup.mixed().notRequired(),

Expand Down