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

🪟 🎉 remove sub-1 hour frequency options #15708

Merged
merged 8 commits into from
Aug 18, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe("Connection main actions", () => {
cy.get("div[data-id='replication-step']").click();

cy.get("div[data-testid='schedule']").click();
cy.get("div[data-testid='Every 5 minutes']").click();
cy.get("div[data-testid='Every hour']").click();
cy.get("button[type=submit]").first().click();
cy.wait("@updateConnection");
cy.get("span[data-id='success-result']").should("exist");
Expand Down
6 changes: 2 additions & 4 deletions airbyte-webapp/src/components/EntityTable/hooks.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getFrequencyConfig } from "config/utils";
import { getFrequencyType } from "config/utils";
import { Action, Namespace } from "core/analytics";
import { buildConnectionUpdate } from "core/domain/connection";
import { useAnalyticsService } from "hooks/services/Analytics";
Expand All @@ -21,14 +21,12 @@ const useSyncActions = (): {
})
);

const frequency = getFrequencyConfig(connection.schedule);

const enabledStreams = connection.syncCatalog.streams.filter((stream) => stream.config?.selected).length;

const trackableAction = connection.status === ConnectionStatus.active ? Action.DISABLE : Action.REENABLE;

analyticsService.track(Namespace.CONNECTION, trackableAction, {
frequency: frequency?.type,
frequency: getFrequencyType(connection.schedule),
connector_source: connection.source?.sourceName,
connector_source_definition_id: connection.source?.sourceDefinitionId,
connector_destination: connection.destination?.destinationName,
Expand Down
76 changes: 0 additions & 76 deletions airbyte-webapp/src/config/FrequencyConfig.json

This file was deleted.

33 changes: 33 additions & 0 deletions airbyte-webapp/src/config/frequencyConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { ConnectionSchedule } from "core/request/AirbyteClient";

export const frequencyConfig: Array<ConnectionSchedule | null> = [
null, // manual
{
units: 1,
timeUnit: "hours",
},
{
units: 2,
timeUnit: "hours",
},
{
units: 3,
timeUnit: "hours",
},
{
units: 6,
timeUnit: "hours",
},
{
units: 8,
timeUnit: "hours",
},
{
units: 12,
timeUnit: "hours",
},
{
units: 24,
timeUnit: "hours",
},
];
6 changes: 2 additions & 4 deletions airbyte-webapp/src/config/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import FrequencyConfig from "config/FrequencyConfig.json";
import { ConnectionSchedule } from "core/request/AirbyteClient";
import { equal } from "utils/objects";

export const getFrequencyConfig = (schedule?: ConnectionSchedule) =>
FrequencyConfig.find((item) => (!schedule && !item.config) || equal(item.config, schedule));
export const getFrequencyType = (schedule?: ConnectionSchedule) =>
schedule ? `${schedule.units} ${schedule.timeUnit}` : "manual";
10 changes: 3 additions & 7 deletions airbyte-webapp/src/hooks/services/useConnectionHook.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { QueryClient, useMutation, useQueryClient } from "react-query";

import { getFrequencyConfig } from "config/utils";
import { getFrequencyType } from "config/utils";
import { Action, Namespace } from "core/analytics";
import { SyncSchema } from "core/domain/catalog";
import { WebBackendConnectionService } from "core/domain/connection";
Expand Down Expand Up @@ -92,15 +92,13 @@ export const useSyncConnection = () => {
const analyticsService = useAnalyticsService();

return useMutation((connection: WebBackendConnectionRead) => {
const frequency = getFrequencyConfig(connection.schedule);

analyticsService.track(Namespace.CONNECTION, Action.SYNC, {
actionDescription: "Manual triggered sync",
connector_source: connection.source?.sourceName,
connector_source_definition_id: connection.source?.sourceDefinitionId,
connector_destination: connection.destination?.destinationName,
connector_destination_definition_id: connection.destination?.destinationDefinitionId,
frequency: frequency?.type,
frequency: getFrequencyType(connection.schedule),
});

return service.sync(connection.connectionId);
Expand Down Expand Up @@ -143,11 +141,9 @@ const useCreateConnection = () => {

const enabledStreams = values.syncCatalog.streams.filter((stream) => stream.config?.selected).length;

const frequencyData = getFrequencyConfig(values.schedule);

analyticsService.track(Namespace.CONNECTION, Action.CREATE, {
actionDescription: "New connection created",
frequency: frequencyData?.type || "",
frequency: getFrequencyType(values.schedule),
connector_source_definition: source?.sourceName,
connector_source_definition_id: sourceDefinition?.sourceDefinitionId,
connector_destination_definition: destination?.destinationName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Navigate, Route, Routes, useParams } from "react-router-dom";
import { LoadingPage, MainPageWithScroll } from "components";
import HeadTitle from "components/HeadTitle";

import { getFrequencyConfig } from "config/utils";
import { getFrequencyType } from "config/utils";
import { Action, Namespace } from "core/analytics";
import { ConnectionStatus } from "core/request/AirbyteClient";
import { useAnalyticsService } from "hooks/services/Analytics";
Expand All @@ -30,16 +30,14 @@ const ConnectionItemPage: React.FC = () => {

const { source, destination } = connection;

const frequency = getFrequencyConfig(connection.schedule);

const onAfterSaveSchema = () => {
analyticsService.track(Namespace.CONNECTION, Action.EDIT_SCHEMA, {
actionDescription: "Connection saved with catalog changes",
connector_source: source.sourceName,
connector_source_definition_id: source.sourceDefinitionId,
connector_destination: destination.destinationName,
connector_destination_definition_id: destination.destinationDefinitionId,
frequency: frequency?.type,
frequency: getFrequencyType(connection.schedule),
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Link } from "react-router-dom";

import ConnectorCard from "components/ConnectorCard";

import { getFrequencyConfig } from "config/utils";
import { getFrequencyType } from "config/utils";
import { ConnectionStatus, SourceRead, DestinationRead, WebBackendConnectionRead } from "core/request/AirbyteClient";
import { FeatureItem, useFeature } from "hooks/services/Feature";
import { RoutePaths } from "pages/routePaths";
Expand All @@ -32,7 +32,6 @@ export const StatusMainInfo: React.FC<StatusMainInfoProps> = ({
const destinationDefinition = useDestinationDefinition(destination.destinationDefinitionId);

const allowSync = useFeature(FeatureItem.AllowSync);
const frequency = getFrequencyConfig(connection.schedule);

const sourceConnectionPath = `../../${RoutePaths.Source}/${source.sourceId}`;
const destinationConnectionPath = `../../${RoutePaths.Destination}/${destination.destinationId}`;
Expand Down Expand Up @@ -64,7 +63,7 @@ export const StatusMainInfo: React.FC<StatusMainInfoProps> = ({
onStatusUpdating={onStatusUpdating}
disabled={!allowSync}
connection={connection}
frequencyType={frequency?.type}
frequencyType={getFrequencyType(connection.schedule)}
/>
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ const ConnectionForm: React.FC<ConnectionFormProps> = ({
);

const errorMessage = submitError ? createFormErrorMessage(submitError) : null;
const frequencies = useFrequencyDropdownData();
const frequencies = useFrequencyDropdownData(connection.schedule);

return (
<Formik
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { renderHook } from "@testing-library/react-hooks";

import { frequencyConfig } from "config/frequencyConfig";
import { ConnectionScheduleTimeUnit } from "core/request/AirbyteClient";
import { TestWrapper as wrapper } from "utils/testutils";

import { useFrequencyDropdownData } from "./formConfig";

describe("useFrequencyDropdownData", () => {
it("should return only default frequencies when no additional frequency is provided", () => {
const { result } = renderHook(() => useFrequencyDropdownData(undefined), { wrapper });
expect(result.current.map((item) => item.value)).toEqual(frequencyConfig);
});

it("should return only default frequencies when additional frequency is already present", () => {
const additionalFrequency = {
units: 1,
timeUnit: ConnectionScheduleTimeUnit["hours"],
};
const { result } = renderHook(() => useFrequencyDropdownData(additionalFrequency), { wrapper });
expect(result.current.map((item) => item.value)).toEqual(frequencyConfig);
});

it("should include additional frequency when provided and unique", () => {
const additionalFrequency = {
units: 7,
timeUnit: ConnectionScheduleTimeUnit["minutes"],
};
const { result } = renderHook(() => useFrequencyDropdownData(additionalFrequency), { wrapper });

expect(result.current.length).toEqual(frequencyConfig.length + 1);
expect(result.current).toContainEqual({ label: "Every 7 minutes", value: { units: 7, timeUnit: "minutes" } });
});
});
45 changes: 28 additions & 17 deletions airbyte-webapp/src/views/Connection/ConnectionForm/formConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as yup from "yup";

import { DropDownRow } from "components";

import FrequencyConfig from "config/FrequencyConfig.json";
import { frequencyConfig } from "config/frequencyConfig";
import { SyncSchema } from "core/domain/catalog";
import {
isDbtTransformation,
Expand Down Expand Up @@ -254,24 +254,35 @@ const useInitialValues = (
}, [initialSchema, connection, isEditMode, destDefinition]);
};

const useFrequencyDropdownData = (): DropDownRow.IDataItem[] => {
const useFrequencyDropdownData = (
additionalFrequency: WebBackendConnectionRead["schedule"]
): DropDownRow.IDataItem[] => {
const { formatMessage } = useIntl();

return useMemo(
() =>
FrequencyConfig.map((item) => ({
value: item.config,
label: item.config
? formatMessage(
{
id: `form.every.${item.config.timeUnit}`,
},
{ value: item.config.units }
)
: formatMessage({ id: "frequency.manual" }),
})),
[formatMessage]
);
return useMemo(() => {
const frequencies = [...frequencyConfig];
if (additionalFrequency) {
const additionalFreqAlreadyPresent = frequencies.some(
(frequency) =>
frequency?.timeUnit === additionalFrequency.timeUnit && frequency?.units === additionalFrequency.units
);
if (!additionalFreqAlreadyPresent) {
frequencies.push(additionalFrequency);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a good use-case for a set
But we'd need to sort on return.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think given that we know our frequency config is rather short (I mean we'll never talk about more than 100 entries here), converting this into a set (and handling the sorting ourselves) sounds like way more effort (even in terms of performance), than just iterating through the (rather small array) once.


return frequencies.map((frequency) => ({
value: frequency,
label: frequency
? formatMessage(
{
id: `form.every.${frequency.timeUnit}`,
},
{ value: frequency.units }
)
: formatMessage({ id: "frequency.manual" }),
}));
}, [formatMessage, additionalFrequency]);
};

export type { ConnectionFormValues, FormikConnectionFormValues };
Expand Down