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

Fix connection settings changing randomly #15332

Merged
merged 2 commits into from
Aug 5, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
@@ -1,4 +1,4 @@
import React, { Suspense, useMemo, useState } from "react";
import React, { Suspense, useMemo } from "react";
import styled from "styled-components";

import { ContentCard } from "components";
Expand All @@ -12,7 +12,6 @@ import { useAnalyticsService } from "hooks/services/Analytics";
import { useCreateConnection, ValuesProps } from "hooks/services/useConnectionHook";
import ConnectionForm from "views/Connection/ConnectionForm";
import { ConnectionFormProps } from "views/Connection/ConnectionForm/ConnectionForm";
import { FormikConnectionFormValues } from "views/Connection/ConnectionForm/formConfig";

import { DestinationRead, SourceRead, WebBackendConnectionRead } from "../../core/request/AirbyteClient";
import { useDiscoverSchema } from "../../hooks/services/useSourceHook";
Expand Down Expand Up @@ -45,21 +44,14 @@ const CreateConnectionContent: React.FC<CreateConnectionContentProps> = ({

const { schema, isLoading, schemaErrorStatus, catalogId, onDiscoverSchema } = useDiscoverSchema(source.sourceId);

const [connectionFormValues, setConnectionFormValues] = useState<FormikConnectionFormValues>();

const connection = useMemo<ConnectionFormProps["connection"]>(
() => ({
name: connectionFormValues?.name ?? "",
namespaceDefinition: connectionFormValues?.namespaceDefinition,
namespaceFormat: connectionFormValues?.namespaceFormat,
prefix: connectionFormValues?.prefix,
schedule: connectionFormValues?.schedule ?? undefined,
syncCatalog: schema,
destination,
source,
catalogId,
}),
[connectionFormValues, schema, destination, source, catalogId]
[schema, destination, source, catalogId]
);

const onSubmitConnectionStep = async (values: ValuesProps) => {
Expand Down Expand Up @@ -124,7 +116,6 @@ const CreateConnectionContent: React.FC<CreateConnectionContentProps> = ({
additionBottomControls={additionBottomControls}
onDropDownSelect={onSelectFrequency}
onSubmit={onSubmitConnectionStep}
onChangeValues={setConnectionFormValues}
/>
</Suspense>
);
Expand Down
3 changes: 3 additions & 0 deletions airbyte-webapp/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@
"connection.sourceTestAgain": "Test source connection again",
"connection.resetData": "Reset your data",
"connection.updateSchema": "Refresh source schema",
"connection.updateSchema.formChanged.title": "Unsaved changes",
"connection.updateSchema.formChanged.text": "Your replication settings have unsaved changes. Those will be lost when refreshing the source schema. Save your changes before refreshing the source schema to not lose them.",
"connection.updateSchema.formChanged.confirm": "Proceed anyway",
timroes marked this conversation as resolved.
Show resolved Hide resolved
"connection.updateSchema.completed": "Refreshed source schema",
"connection.updateSchema.confirm": "Confirm",
"connection.updateSchema.new": "{value} new {item}",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { faSyncAlt } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import React, { useMemo, useState } from "react";
import React, { useCallback, useRef, useState } from "react";
import { FormattedMessage, useIntl } from "react-intl";
import { useAsyncFn, useUnmount } from "react-use";
import styled from "styled-components";
Expand All @@ -10,6 +10,7 @@ import LoadingSchema from "components/LoadingSchema";

import { toWebBackendConnectionUpdate } from "core/domain/connection";
import { ConnectionStateType, ConnectionStatus } from "core/request/AirbyteClient";
import { useConfirmationModalService } from "hooks/services/ConfirmationModal";
import { useModalService } from "hooks/services/Modal";
import {
useConnectionLoad,
Expand All @@ -21,7 +22,6 @@ import { equal } from "utils/objects";
import { CatalogDiffModal } from "views/Connection/CatalogDiffModal/CatalogDiffModal";
import ConnectionForm from "views/Connection/ConnectionForm";
import { ConnectionFormSubmitResult } from "views/Connection/ConnectionForm/ConnectionForm";
import { FormikConnectionFormValues } from "views/Connection/ConnectionForm/formConfig";

interface ReplicationViewProps {
onAfterSaveSchema: () => void;
Expand Down Expand Up @@ -84,9 +84,10 @@ const TryArrow = styled(FontAwesomeIcon)`
export const ReplicationView: React.FC<ReplicationViewProps> = ({ onAfterSaveSchema, connectionId }) => {
const { formatMessage } = useIntl();
const { openModal, closeModal } = useModalService();
const { openConfirmationModal, closeConfirmationModal } = useConfirmationModalService();
const connectionFormDirtyRef = useRef<boolean>(false);
const [activeUpdatingSchemaMode, setActiveUpdatingSchemaMode] = useState(false);
const [saved, setSaved] = useState(false);
const [connectionFormValues, setConnectionFormValues] = useState<FormikConnectionFormValues>();
const connectionService = useConnectionService();
const { mutateAsync: updateConnection } = useUpdateConnection();

Expand All @@ -97,28 +98,19 @@ export const ReplicationView: React.FC<ReplicationViewProps> = ({ onAfterSaveSch
[connectionId]
);

useUnmount(() => closeModal());

const connection = useMemo(() => {
if (activeUpdatingSchemaMode && connectionWithRefreshCatalog) {
// merge connectionFormValues (unsaved previous form state) with the refreshed connection data:
// 1. if there is a namespace definition, format, prefix, or schedule in connectionFormValues,
// use those and fill in the rest from the database
// 2. otherwise, use the values from the database
// 3. if none of the above, use the default values.
return {
...connectionWithRefreshCatalog,
namespaceDefinition:
connectionFormValues?.namespaceDefinition ?? connectionWithRefreshCatalog.namespaceDefinition,
namespaceFormat: connectionFormValues?.namespaceFormat ?? connectionWithRefreshCatalog.namespaceFormat,
prefix: connectionFormValues?.prefix ?? connectionWithRefreshCatalog.prefix,
schedule: connectionFormValues?.schedule ?? connectionWithRefreshCatalog.schedule,
};
}
return initialConnection;
}, [activeUpdatingSchemaMode, connectionWithRefreshCatalog, initialConnection, connectionFormValues]);
useUnmount(() => {
closeModal();
closeConfirmationModal();
});

const connection = activeUpdatingSchemaMode ? connectionWithRefreshCatalog : initialConnection;

const saveConnection = async (values: ValuesProps, { skipReset }: { skipReset: boolean }) => {
if (!connection) {
// onSubmit should only be called while the catalog isn't currently refreshing at the moment,
// which is the only case when `connection` would be `undefined`.
return;
}
const initialSyncSchema = connection.syncCatalog;
const connectionAsUpdate = toWebBackendConnectionUpdate(connection);

Expand Down Expand Up @@ -174,7 +166,7 @@ export const ReplicationView: React.FC<ReplicationViewProps> = ({ onAfterSaveSch
}
};

const onRefreshSourceSchema = async () => {
const refreshSourceSchema = async () => {
setSaved(false);
setActiveUpdatingSchemaMode(true);
const { catalogDiff, syncCatalog } = await refreshCatalog();
Expand All @@ -189,11 +181,33 @@ export const ReplicationView: React.FC<ReplicationViewProps> = ({ onAfterSaveSch
}
};

const onRefreshSourceSchema = async () => {
if (connectionFormDirtyRef.current) {
// The form is dirty so we show a warning before proceeding.
openConfirmationModal({
title: "connection.updateSchema.formChanged.title",
text: "connection.updateSchema.formChanged.text",
submitButtonText: "connection.updateSchema.formChanged.confirm",
onSubmit: () => {
closeConfirmationModal();
refreshSourceSchema();
},
});
} else {
// The form is not dirty so we can directly refresh the source schema.
refreshSourceSchema();
}
};

const onCancelConnectionFormEdit = () => {
setSaved(false);
setActiveUpdatingSchemaMode(false);
};

const onDirtyChanges = useCallback((dirty: boolean) => {
connectionFormDirtyRef.current = dirty;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ We track the dirty state in this component in a ref not state, since we don't require any rerendering of this component if this changes.

}, []);

return (
<Content>
{!isRefreshingCatalog && connection ? (
Expand All @@ -210,7 +224,7 @@ export const ReplicationView: React.FC<ReplicationViewProps> = ({ onAfterSaveSch
<FormattedMessage id="connection.updateSchema" />
</Button>
}
onChangeValues={setConnectionFormValues}
onFormDirtyChanges={onDirtyChanges}
/>
) : (
<LoadingSchema />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { Field, FieldProps, Form, Formik, FormikHelpers, useFormikContext } from "formik";
import React, { useCallback, useState } from "react";
import { Field, FieldProps, Form, Formik, FormikHelpers } from "formik";
import React, { useCallback, useEffect, useState } from "react";
import { FormattedMessage, useIntl } from "react-intl";
import { useToggle } from "react-use";
import { useDebounce } from "react-use";
import styled from "styled-components";

import { Card, ControlLabels, DropDown, DropDownRow, H5, Input } from "components";
Expand Down Expand Up @@ -102,28 +101,26 @@ export interface ConnectionFormSubmitResult {

export type ConnectionFormMode = "create" | "edit" | "readonly";

// eslint-disable-next-line react/function-component-definition
function FormValuesChangeTracker<T>({ onChangeValues }: { onChangeValues?: (values: T) => void }) {
// Grab values from context
const { values } = useFormikContext<T>();
useDebounce(
() => {
onChangeValues?.(values);
},
200,
[values, onChangeValues]
);
return null;
interface DirtyChangeTrackerProps {
dirty: boolean;
onChanges: (dirty: boolean) => void;
}

const DirtyChangeTracker: React.FC<DirtyChangeTrackerProps> = ({ dirty, onChanges }) => {
useEffect(() => {
onChanges(dirty);
}, [dirty, onChanges]);
return null;
};

interface ConnectionFormProps {
onSubmit: (values: ConnectionFormValues) => Promise<ConnectionFormSubmitResult | void>;
className?: string;
additionBottomControls?: React.ReactNode;
successMessage?: React.ReactNode;
onDropDownSelect?: (item: DropDownRow.IDataItem) => void;
onCancel?: () => void;
onChangeValues?: (values: FormikConnectionFormValues) => void;
onFormDirtyChanges?: (dirty: boolean) => void;

/** Should be passed when connection is updated with withRefreshCatalog flag */
canSubmitUntouchedForm?: boolean;
Expand All @@ -146,7 +143,7 @@ const ConnectionForm: React.FC<ConnectionFormProps> = ({
canSubmitUntouchedForm,
additionalSchemaControl,
connection,
onChangeValues,
onFormDirtyChanges,
}) => {
const destDefinition = useGetDestinationDefinitionSpecification(connection.destination.destinationDefinitionId);
const { clearFormChange } = useFormChangeTrackerService();
Expand Down Expand Up @@ -200,7 +197,7 @@ const ConnectionForm: React.FC<ConnectionFormProps> = ({
{({ isSubmitting, setFieldValue, isValid, dirty, resetForm, values }) => (
<FormContainer className={className}>
<FormChangeTracker changed={dirty} formId={formId} />
<FormValuesChangeTracker onChangeValues={onChangeValues} />
{onFormDirtyChanges && <DirtyChangeTracker dirty={dirty} onChanges={onFormDirtyChanges} />}
{!isEditMode && (
<Section>
<Field name="name">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ const useInitialValues = (
const initialValues: FormikConnectionFormValues = {
name: connection.name ?? `${connection.source.name} <> ${connection.destination.name}`,
syncCatalog: initialSchema,
schedule: connection.connectionId || connection.schedule ? connection.schedule ?? null : DEFAULT_SCHEDULE,
schedule: connection.connectionId ? connection.schedule ?? null : DEFAULT_SCHEDULE,
prefix: connection.prefix || "",
namespaceDefinition: connection.namespaceDefinition || NamespaceDefinitionType.source,
namespaceFormat: connection.namespaceFormat ?? SOURCE_NAMESPACE_TAG,
Expand Down