Skip to content

Commit

Permalink
🪟 🔧 Move auto-detect schema changes feature flag to FeatureService (#…
Browse files Browse the repository at this point in the history
…20232)

* Move auto-detect schema changes feature flag to FeatureService

* Remove connection.autoDetectSchemaChanges experiment constant

* Fix tests using old autoDetectSchemaChanges flag
  • Loading branch information
edmundito authored Dec 12, 2022
1 parent 2dbbe65 commit d374b85
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 51 deletions.
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 @@ -83,13 +83,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 @@ -131,7 +131,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

0 comments on commit d374b85

Please sign in to comment.