🪟 🐛 Schedule type cron bug fix (airbytehq#17058)

* remove inaccurate and unneeded setting of schedule type * Fixing & adding tests Co-authored-by: KC <krishna@airbyte.io>
robbinhan · Sep 29, 2022 · 61873e2 · 61873e2
1 parent 949e61e
commit 61873e2
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 7 deletions.
diff --git a/airbyte-webapp/src/hooks/services/Connection/ConnectionFormService.test.tsx b/airbyte-webapp/src/hooks/services/Connection/ConnectionFormService.test.tsx
@@ -8,7 +8,7 @@ import mockDest from "test-utils/mock-data/mockDestinationDefinition.json";
 import mockWorkspace from "test-utils/mock-data/mockWorkspace.json";
 import { TestWrapper } from "test-utils/testutils";
 
-import { WebBackendConnectionRead } from "core/request/AirbyteClient";
+import { ConnectionScheduleType, WebBackendConnectionRead } from "core/request/AirbyteClient";
 
 import { ModalCancel } from "../Modal";
 import {
@@ -74,7 +74,12 @@ describe("ConnectionFormService", () => {
     expect(resetForm).toBeCalledWith({ values: testValues });
     expect(onSubmit).toBeCalledWith({
       operations: [],
-      scheduleType: "manual",
+      scheduleData: {
+        cron: {
+          cronExpression: undefined,
+          cronTimeZone: undefined,
+        },
+      },
       syncCatalog: {
         streams: undefined,
       },
@@ -83,6 +88,59 @@ describe("ConnectionFormService", () => {
     expect(result.current.errorMessage).toBe(null);
   });
 
+  const expectation = {
+    [ConnectionScheduleType.basic]: {
+      basicSchedule: {
+        timeUnit: undefined,
+        units: undefined,
+      },
+    },
+    [ConnectionScheduleType.manual]: undefined,
+    [ConnectionScheduleType.cron]: {
+      cron: {
+        cronExpression: undefined,
+        cronTimeZone: undefined,
+      },
+    },
+  };
+
+  Object.values(ConnectionScheduleType).forEach((scheduleType) => {
+    it(`should return expected results when onSubmit is called with ${scheduleType}`, async () => {
+      const { result } = renderHook(useConnectionFormService, {
+        wrapper: Wrapper,
+        initialProps: {
+          connection: mockConnection as WebBackendConnectionRead,
+          mode: "create",
+          formId: Math.random().toString(),
+          onSubmit,
+          onAfterSubmit,
+          onCancel,
+          formDirty: false,
+        },
+      });
+
+      const resetForm = jest.fn();
+      const testValues: any = {
+        scheduleType,
+      };
+      await act(async () => {
+        await result.current.onFormSubmit(testValues, { resetForm } as any);
+      });
+
+      expect(resetForm).toBeCalledWith({ values: testValues });
+      expect(onSubmit).toBeCalledWith({
+        operations: [],
+        scheduleData: expectation[scheduleType],
+        scheduleType,
+        syncCatalog: {
+          streams: undefined,
+        },
+      });
+      expect(onAfterSubmit).toBeCalledWith();
+      expect(result.current.errorMessage).toBe(null);
+    });
+  });
+
   it("should catch if onSubmit throws and generate an error message", async () => {
     const errorMessage = "asdf";
     onSubmit.mockImplementation(async () => {

diff --git a/airbyte-webapp/src/hooks/services/Connection/ConnectionFormService.tsx b/airbyte-webapp/src/hooks/services/Connection/ConnectionFormService.tsx
@@ -53,11 +53,6 @@ const useConnectionForm = ({
 
   const onFormSubmit = useCallback(
     async (values: FormikConnectionFormValues, formikHelpers: FormikHelpers<FormikConnectionFormValues>) => {
-      // Set the scheduleType based on the schedule value
-      values.scheduleType = values.scheduleData?.basicSchedule
-        ? ConnectionScheduleType.basic
-        : ConnectionScheduleType.manual;
-
       // TODO: We should align these types
       // With the PATCH-style endpoint available we might be able to forego this pattern
       const formValues: ConnectionFormValues = connectionValidationSchema.cast(values, {