Skip to content

Commit

Permalink
🪟 Per-Stream state new flow (#14634)
Browse files Browse the repository at this point in the history
* [WIP] new per stream flow

* Start work on ModalSerice

* Continue work

* Fix typo

* Change wording

* Add todos and remove dead code

* Remove dead message

* Adjust to new API

* Adjust to new modal changes

* Remove debug output

* Fix e2e test

* Add data-testids

* Adjust for PR review

* Switch to new ModalFooter/Body components

* Add ModalService tests

* Downgrade user-events again

* Only compare selected streams

* Update airbyte-webapp/src/locales/en.json

Co-authored-by: Andy Jih <andyjih@users.noreply.github.com>

* Update airbyte-webapp/src/locales/en.json

Co-authored-by: Andy Jih <andyjih@users.noreply.github.com>

* Remove redundant space

Co-authored-by: Andy Jih <andyjih@users.noreply.github.com>
  • Loading branch information
2 people authored and mfsiega-airbyte committed Jul 21, 2022
1 parent 823a826 commit 54810d0
Show file tree
Hide file tree
Showing 17 changed files with 325 additions and 173 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe("Connection main actions", () => {
});

it("Update connection", () => {
cy.intercept("/api/v1/web_backend/connections/update").as("updateConnection");
cy.intercept("/api/v1/web_backend/connections/updateNew").as("updateConnection");

createTestConnection("Test update connection source cypress", "Test update connection destination cypress");

Expand Down
9 changes: 6 additions & 3 deletions airbyte-webapp/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ServicesProvider } from "core/servicesProvider";
import { ConfirmationModalService } from "hooks/services/ConfirmationModal";
import { defaultFeatures, FeatureService } from "hooks/services/Feature";
import { FormChangeTrackerService } from "hooks/services/FormChangeTracker";
import { ModalServiceProvider } from "hooks/services/Modal";
import NotificationService from "hooks/services/Notification";
import { AnalyticsProvider } from "views/common/AnalyticsProvider";
import { StoreProvider } from "views/common/StoreProvider";
Expand Down Expand Up @@ -44,9 +45,11 @@ const Services: React.FC = ({ children }) => (
<FeatureService features={defaultFeatures}>
<NotificationService>
<ConfirmationModalService>
<FormChangeTrackerService>
<ApiServices>{children}</ApiServices>
</FormChangeTrackerService>
<ModalServiceProvider>
<FormChangeTrackerService>
<ApiServices>{children}</ApiServices>
</FormChangeTrackerService>
</ModalServiceProvider>
</ConfirmationModalService>
</NotificationService>
</FeatureService>
Expand Down
6 changes: 3 additions & 3 deletions airbyte-webapp/src/components/Modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ const cardStyleBySize = {
};

const Modal: React.FC<ModalProps> = ({ children, title, onClose, clear, closeOnBackground, size }) => {
const handleUserKeyPress = useCallback((event, closeModal) => {
const { keyCode } = event;
const handleUserKeyPress = useCallback((event: KeyboardEvent, closeModal: () => void) => {
const { key } = event;
// Escape key
if (keyCode === 27) {
if (key === "Escape") {
closeModal();
}
}, []);
Expand Down
2 changes: 1 addition & 1 deletion airbyte-webapp/src/components/Modal/ModalBody.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

.modalBody {
padding: variables.$spacing-lg variables.$spacing-xl;
overflow: scroll;
overflow: auto;
max-width: 100%;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { deleteConnection, resetConnection, syncConnection, getState } from "../../request/AirbyteClient";
import { deleteConnection, resetConnection, syncConnection, getState, getStateType } from "../../request/AirbyteClient";
import { AirbyteRequestService } from "../../request/AirbyteRequestService";

export class ConnectionService extends AirbyteRequestService {
Expand All @@ -17,4 +17,8 @@ export class ConnectionService extends AirbyteRequestService {
public getState(connectionId: string) {
return getState({ connectionId }, this.requestOptions);
}

public getStateType(connectionId: string) {
return getStateType({ connectionId }, this.requestOptions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
webBackendCreateConnection,
webBackendGetConnection,
webBackendListConnectionsForWorkspace,
webBackendUpdateConnection,
webBackendUpdateConnectionNew as webBackendUpdateConnection,
} from "../../request/AirbyteClient";
import { AirbyteRequestService } from "../../request/AirbyteRequestService";

Expand Down
1 change: 1 addition & 0 deletions airbyte-webapp/src/core/domain/connection/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const buildConnectionUpdate = (
connection: WebBackendConnectionRead,
connectionUpdate: Partial<WebBackendConnectionUpdate>
): WebBackendConnectionUpdate => ({
skipReset: true,
...toWebBackendConnectionUpdate(connection),
...connectionUpdate,
});
87 changes: 87 additions & 0 deletions airbyte-webapp/src/hooks/services/Modal/ModalService.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { render, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { useEffectOnce } from "react-use";

import { ModalServiceProvider, useModalService } from "./ModalService";
import { ModalResult } from "./types";

const TestComponent: React.FC<{ onModalResult?: (result: ModalResult<unknown>) => void }> = ({ onModalResult }) => {
const { openModal } = useModalService();
useEffectOnce(() => {
openModal({
title: "Test Modal Title",
content: ({ onCancel, onClose }) => (
<div data-testid="testModalContent">
<button onClick={onCancel} data-testid="cancel">
Cancel
</button>
<button onClick={() => onClose("reason1")} data-testid="close-reason1">
Close Reason 1
</button>
<button onClick={() => onClose("reason2")} data-testid="close-reason2">
Close Reason 2
</button>
</div>
),
}).then(onModalResult);
});
return null;
};

const renderModal = (resultCallback?: (reason: unknown) => void) => {
return render(
<ModalServiceProvider>
<TestComponent onModalResult={resultCallback} />
</ModalServiceProvider>
);
};

describe("ModalService", () => {
it("should open a modal on openModal", () => {
const rendered = renderModal();

expect(rendered.getByText("Test Modal Title")).toBeTruthy();
expect(rendered.getByTestId("testModalContent")).toBeTruthy();
});

it("should close the modal with escape and emit a cancel result", async () => {
const resultCallback = jest.fn();

const rendered = renderModal(resultCallback);

await waitFor(() => userEvent.keyboard("{Escape}"));

expect(rendered.queryByTestId("testModalContent")).toBeFalsy();
expect(resultCallback).toHaveBeenCalledWith({ type: "canceled" });
});

it("should allow cancelling the modal from inside", async () => {
const resultCallback = jest.fn();

const rendered = renderModal(resultCallback);

await waitFor(() => userEvent.click(rendered.getByTestId("cancel")));

expect(rendered.queryByTestId("testModalContent")).toBeFalsy();
expect(resultCallback).toHaveBeenCalledWith({ type: "canceled" });
});

it("should allow closing the button with a reason and return that reason", async () => {
const resultCallback = jest.fn();

let rendered = renderModal(resultCallback);

await waitFor(() => userEvent.click(rendered.getByTestId("close-reason1")));

expect(rendered.queryByTestId("testModalContent")).toBeFalsy();
expect(resultCallback).toHaveBeenCalledWith({ type: "closed", reason: "reason1" });

resultCallback.mockReset();
rendered = renderModal(resultCallback);

await waitFor(() => userEvent.click(rendered.getByTestId("close-reason2")));

expect(rendered.queryByTestId("testModalContent")).toBeFalsy();
expect(resultCallback).toHaveBeenCalledWith({ type: "closed", reason: "reason2" });
});
});
64 changes: 64 additions & 0 deletions airbyte-webapp/src/hooks/services/Modal/ModalService.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import React, { useContext, useMemo, useRef, useState } from "react";
import { firstValueFrom, Subject } from "rxjs";

import { Modal } from "components";

import { ModalOptions, ModalResult, ModalServiceContext } from "./types";

const modalServiceContext = React.createContext<ModalServiceContext | undefined>(undefined);

export const ModalServiceProvider: React.FC = ({ children }) => {
// The any here is due to the fact, that every call to open a modal might come in with
// a different type, thus we can't type this with unknown or a generic.
// The consuming code of this service though is properly typed, so that this `any` stays
// encapsulated within this component.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const [modalOptions, setModalOptions] = useState<ModalOptions<any>>();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const resultSubjectRef = useRef<Subject<ModalResult<any>>>();

const service: ModalServiceContext = useMemo(
() => ({
openModal: (options) => {
resultSubjectRef.current = new Subject();
setModalOptions(options);

return firstValueFrom(resultSubjectRef.current).then((reason) => {
setModalOptions(undefined);
resultSubjectRef.current = undefined;
return reason;
});
},
closeModal: () => {
resultSubjectRef.current?.next({ type: "canceled" });
},
}),
[]
);

return (
<modalServiceContext.Provider value={service}>
{children}
{modalOptions && (
<Modal
title={modalOptions.title}
size={modalOptions.size}
onClose={() => resultSubjectRef.current?.next({ type: "canceled" })}
>
<modalOptions.content
onCancel={() => resultSubjectRef.current?.next({ type: "canceled" })}
onClose={(reason) => resultSubjectRef.current?.next({ type: "closed", reason })}
/>
</Modal>
)}
</modalServiceContext.Provider>
);
};

export const useModalService = () => {
const context = useContext(modalServiceContext);
if (!context) {
throw new Error("Can't use ModalService outside ModalServiceProvider");
}
return context;
};
1 change: 1 addition & 0 deletions airbyte-webapp/src/hooks/services/Modal/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./ModalService";
21 changes: 21 additions & 0 deletions airbyte-webapp/src/hooks/services/Modal/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import React from "react";

import { ModalProps } from "components/Modal/Modal";

export interface ModalOptions<T> {
title: ModalProps["title"];
size?: ModalProps["size"];
content: React.ComponentType<ModalContentProps<T>>;
}

export type ModalResult<T> = { type: "canceled" } | { type: "closed"; reason: T };

interface ModalContentProps<T> {
onClose: (reason: T) => void;
onCancel: () => void;
}

export interface ModalServiceContext {
openModal: <ResultType>(options: ModalOptions<ResultType>) => Promise<ModalResult<ResultType>>;
closeModal: () => void;
}
19 changes: 5 additions & 14 deletions airbyte-webapp/src/hooks/services/useConnectionHook.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function useWebConnectionService() {
);
}

function useConnectionService() {
export function useConnectionService() {
const config = useConfig();
const middlewares = useDefaultRequestMiddlewares();
return useInitService(() => new ConnectionService(config.apiUrl, middlewares), [config.apiUrl, middlewares]);
Expand Down Expand Up @@ -190,20 +190,11 @@ const useUpdateConnection = () => {
const service = useWebConnectionService();
const queryClient = useQueryClient();

return useMutation(
(connectionUpdate: WebBackendConnectionUpdate) => {
const withRefreshedCatalogCleaned = connectionUpdate.withRefreshedCatalog
? { withRefreshedCatalog: connectionUpdate.withRefreshedCatalog }
: null;

return service.update({ ...connectionUpdate, ...withRefreshedCatalogCleaned });
return useMutation((connectionUpdate: WebBackendConnectionUpdate) => service.update(connectionUpdate), {
onSuccess: (connection) => {
queryClient.setQueryData(connectionsKeys.detail(connection.connectionId), connection);
},
{
onSuccess: (connection) => {
queryClient.setQueryData(connectionsKeys.detail(connection.connectionId), connection);
},
}
);
});
};

const useConnectionList = (): ListConnection => {
Expand Down
12 changes: 6 additions & 6 deletions airbyte-webapp/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
"form.noNeed": "No need!",
"form.reset": "Reset",
"form.resetData": "Reset your data",
"form.changedColumns": "You have changed which column is used to detect new records for a stream. You may want to reset the data in your stream.",
"form.resetDataText": "Resetting your data will delete all the data for this connection in your destination and start syncs from scratch. Are you sure you want to do this?",
"form.dockerError": "Could not find docker image",
"form.edit": "Edit",
Expand All @@ -106,8 +105,6 @@
"form.destinationConnector": "Destination connector",
"form.addItems": "Add",
"form.items": "Items",
"form.toSaveSchema": "To save the new schema, click on Save changes.",
"form.noteStartSync": "Note that it will delete all the data in your destination and start syncing from scratch. ",
"form.pkSelected": "{count, plural, =0 { } one {{items}} other {# keys selected}}",
"form.url.error": "field must be a valid URL",
"form.setupGuide": "Setup Guide",
Expand Down Expand Up @@ -317,7 +314,12 @@
"syncMode.full_refresh": "Full refresh",
"syncMode.incremental": "Incremental",

"connection.warningUpdateSchema": "WARNING! Updating the schema will delete all the data for this connection in your destination and start syncing from scratch.",
"connection.resetModalTitle": "Stream configuration changed",
"connection.streamResetHint": "Due to changes in the stream configuration, we recommend a data reset. A reset will delete data in the destination of the affected streams and then re-sync that data. Skipping the reset is discouraged and might lead to unexpected behavior.",
"connection.streamFullResetHint": "Due to changes in the stream configuration, we recommend a data reset. A reset will delete data in the destination of the affected streams and then re-sync that data. Skipping the reset is discouraged and might lead to unexpected behavior.",
"connection.saveWithReset": "Reset affected streams (recommended)",
"connection.saveWithFullReset": "Reset all streams (recommended)",
"connection.save": "Save connection",
"connection.title": "Connection",
"connection.description": "<b>Connections</b> link Sources to Destinations.",
"connection.fromTo": "{source} → {destination}",
Expand All @@ -326,12 +328,10 @@
"connection.testsFailed": "The connection tests failed.",
"connection.destinationCheckSettings": "Check destination settings",
"connection.sourceCheckSettings": "Check source settings",
"connection.saveAndReset": "Save changes & reset data",
"connection.destinationTestAgain": "Test destination connection again",
"connection.sourceTestAgain": "Test source connection again",
"connection.resetData": "Reset your data",
"connection.updateSchema": "Refresh source schema",
"connection.updateSchemaText": "WARNING! Updating the schema will delete all the data for this connection in your destination and start syncing from scratch. Are you sure you want to do this?",
"connection.newConnection": "+ New connection",
"connection.newConnectionTitle": "New connection",
"connection.noConnections": "Connection list is empty",
Expand Down
25 changes: 14 additions & 11 deletions airbyte-webapp/src/packages/cloud/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { I18nProvider } from "core/i18n";
import { ConfirmationModalService } from "hooks/services/ConfirmationModal";
import { FeatureItem, FeatureService } from "hooks/services/Feature";
import { FormChangeTrackerService } from "hooks/services/FormChangeTracker";
import { ModalServiceProvider } from "hooks/services/Modal";
import NotificationServiceProvider from "hooks/services/Notification";
import en from "locales/en.json";
import { Routing } from "packages/cloud/cloudRoutes";
Expand Down Expand Up @@ -37,17 +38,19 @@ const Services: React.FC = ({ children }) => (
<ApiErrorBoundary>
<NotificationServiceProvider>
<ConfirmationModalService>
<FormChangeTrackerService>
<FeatureService
features={[FeatureItem.AllowOAuthConnector, FeatureItem.AllowCreateConnection, FeatureItem.AllowSync]}
>
<AppServicesProvider>
<AuthenticationProvider>
<IntercomProvider>{children}</IntercomProvider>
</AuthenticationProvider>
</AppServicesProvider>
</FeatureService>
</FormChangeTrackerService>
<ModalServiceProvider>
<FormChangeTrackerService>
<FeatureService
features={[FeatureItem.AllowOAuthConnector, FeatureItem.AllowCreateConnection, FeatureItem.AllowSync]}
>
<AppServicesProvider>
<AuthenticationProvider>
<IntercomProvider>{children}</IntercomProvider>
</AuthenticationProvider>
</AppServicesProvider>
</FeatureService>
</FormChangeTrackerService>
</ModalServiceProvider>
</ConfirmationModalService>
</NotificationServiceProvider>
</ApiErrorBoundary>
Expand Down
Loading

0 comments on commit 54810d0

Please sign in to comment.