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

🪟 🎉 Multi-cloud: edit connection & workspace data geographies in the UI #18611

Merged
merged 29 commits into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e3c4e53
add geographies service and basic dropdown
josephkmh Oct 27, 2022
511d2e3
add feature for changing data geography
josephkmh Oct 28, 2022
3396376
revert change to connectionformfields
josephkmh Oct 28, 2022
790520c
add default data residency to workspace settings
josephkmh Oct 28, 2022
2002be5
add data residency card to connection creation
josephkmh Oct 28, 2022
59efe5d
add data residency editing to conneciton settings tab
josephkmh Oct 28, 2022
5c6c347
fix styling to match figma
josephkmh Oct 28, 2022
52266f6
update workspace mock
josephkmh Oct 28, 2022
2c13e99
fix formConfig snapshots
josephkmh Oct 28, 2022
b37db85
mock workspace for conneciton edit tests
josephkmh Oct 28, 2022
231182a
mock workspaces service for connection replication tab tests
josephkmh Oct 28, 2022
496f5cd
Merge branch 'master' into joey/multi-cloud-connection-geography
josephkmh Oct 28, 2022
e84242a
fix page tracking code
josephkmh Oct 28, 2022
4355e4e
fix notification id
josephkmh Oct 31, 2022
c8648f6
undo unrelated css change
josephkmh Oct 31, 2022
8c5f984
Merge branch 'master' into joey/multi-cloud-connection-geography
josephkmh Nov 2, 2022
2e82f61
updated copy
josephkmh Nov 2, 2022
2cbf4d4
update docs links
josephkmh Nov 2, 2022
cbbef13
move & rename connection geography component
josephkmh Nov 3, 2022
1ba3167
Merge branch 'master' into joey/multi-cloud-connection-geography
josephkmh Nov 3, 2022
a62ff9b
pr review: drop "default" from "default data residency"
josephkmh Nov 3, 2022
cf88059
pr feedback: docs links
josephkmh Nov 3, 2022
10fbc49
pr feedback: simplify expression
josephkmh Nov 3, 2022
c15946a
adjust menu item label
josephkmh Nov 3, 2022
bf21502
simplify type
josephkmh Nov 3, 2022
71db34a
update string
josephkmh Nov 4, 2022
c7d0fe4
add defaultMessage fallback
josephkmh Nov 4, 2022
eb44d45
adjust capitalization
josephkmh Nov 4, 2022
27edabf
delete unused string
josephkmh Nov 4, 2022
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
Expand Up @@ -25,6 +25,7 @@ import {

import styles from "./CreateConnectionForm.module.scss";
import { CreateConnectionNameField } from "./CreateConnectionNameField";
import { DataResidency } from "./DataResidency";
import { SchemaError } from "./SchemaError";

interface CreateConnectionProps {
Expand All @@ -39,7 +40,7 @@ interface CreateConnectionPropsInner extends Pick<CreateConnectionProps, "afterS

const CreateConnectionFormInner: React.FC<CreateConnectionPropsInner> = ({ schemaError, afterSubmitConnection }) => {
const navigate = useNavigate();

const canEditDataGeographies = useFeature(FeatureItem.AllowChangeDataGeographies);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is false for now, but can be overridden in LaunchDarkly for testing

const { mutateAsync: createConnection } = useCreateConnection();

const { clearFormChange } = useFormChangeTrackerService();
Expand Down Expand Up @@ -113,6 +114,7 @@ const CreateConnectionFormInner: React.FC<CreateConnectionPropsInner> = ({ schem
{({ values, isSubmitting, isValid, dirty }) => (
<Form>
<CreateConnectionNameField />
{canEditDataGeographies && <DataResidency />}
<ConnectionFormFields values={values} isSubmitting={isSubmitting} dirty={dirty} />
<OperationsSection
onStartEditTransformation={() => setEditingTransformation(true)}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@use "scss/variables";

.flexRow {
display: flex;
flex-direction: row;
justify-content: flex-start;
align-items: flex-start;
gap: variables.$spacing-md;
}

.leftFieldCol {
flex: 1;
max-width: 640px;
padding-right: 30px;
}

.rightFieldCol {
flex: 1;
max-width: 300px;
}
61 changes: 61 additions & 0 deletions airbyte-webapp/src/components/CreateConnection/DataResidency.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Field, FieldProps, useFormikContext } from "formik";
import { FormattedMessage, useIntl } from "react-intl";

import { ControlLabels } from "components/LabeledControl";
import { DropDown } from "components/ui/DropDown";

import { useAvailableGeographies } from "packages/cloud/services/geographies/GeographiesService";
import { links } from "utils/links";
import { Section } from "views/Connection/ConnectionForm/components/Section";

import styles from "./DataResidency.module.scss";

interface DataResidencyProps {
name?: string;
}

export const DataResidency: React.FC<DataResidencyProps> = ({ name = "geography" }) => {
const { formatMessage } = useIntl();
const { setFieldValue } = useFormikContext();
const { geographies } = useAvailableGeographies();

return (
<Section title={formatMessage({ id: "connection.geographyTitle" })}>
<Field name={name}>
{({ field, form }: FieldProps<string>) => (
<div className={styles.flexRow}>
<div className={styles.leftFieldCol}>
<ControlLabels
nextLine
label={<FormattedMessage id="connection.geographyTitle" />}
message={
<FormattedMessage
id="connection.geographyDescription"
values={{
lnk: (node: React.ReactNode) => (
<a href={links.cloudAllowlistIPsLink} target="_blank" rel="noreferrer">
{node}
</a>
),
}}
/>
}
/>
</div>
<div className={styles.rightFieldCol}>
<DropDown
isDisabled={form.isSubmitting}
options={geographies.map((geography) => ({
label: formatMessage({ id: `connection.geography.${geography}` }),
value: geography,
}))}
value={field.value}
onChange={(geography) => setFieldValue(name, geography.value)}
/>
</div>
</div>
)}
</Field>
</Section>
);
};
20 changes: 10 additions & 10 deletions airbyte-webapp/src/components/Label/Label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,18 @@ interface IProps {
nextLine?: boolean;
success?: boolean;
message?: string | React.ReactNode;
additionLength?: number;
className?: string;
onClick?: (data: unknown) => void;
htmlFor?: string;
}

const Content = styled.label<{ additionLength?: number | string }>`
const Content = styled.label`
display: block;
font-weight: 500;
font-size: 14px;
line-height: 17px;
color: ${({ theme }) => theme.textColor};
padding-bottom: 5px;
width: calc(100% + ${({ additionLength }) => (additionLength === 0 || additionLength ? additionLength : 30)}px);

& a {
text-decoration: underline;
Expand All @@ -31,16 +29,18 @@ const MessageText = styled.span<Pick<IProps, "error" | "success">>`
white-space: break-spaces;
color: ${(props) =>
props.error ? props.theme.dangerColor : props.success ? props.theme.successColor : props.theme.greyColor40};
font-size: 13px;
font-size: 12px;
font-weight: 400;

a:link,
a:hover,
a:visited {
color: ${(props) => props.theme.greyColor40};
}
`;

const Label: React.FC<React.PropsWithChildren<IProps>> = (props) => (
<Content
additionLength={props.additionLength}
className={props.className}
onClick={props.onClick}
htmlFor={props.htmlFor}
>
<Content className={props.className} onClick={props.onClick} htmlFor={props.htmlFor}>
{props.children}
{props.message && (
<span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export interface ControlLabelsProps {
success?: boolean;
nextLine?: boolean;
message?: React.ReactNode;
labelAdditionLength?: number;
label?: React.ReactNode;
infoTooltipContent?: React.ReactNode;
optional?: boolean;
Expand All @@ -27,7 +26,6 @@ const ControlLabels: React.FC<React.PropsWithChildren<ControlLabelsProps>> = (pr
error={props.error}
success={props.success}
message={props.message}
additionLength={props.labelAdditionLength}
nextLine={props.nextLine}
htmlFor={props.htmlFor}
>
Expand Down
21 changes: 3 additions & 18 deletions airbyte-webapp/src/components/LabeledInput/LabeledInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,11 @@ import React from "react";
import { ControlLabels, ControlLabelsProps } from "components/LabeledControl";
import { Input, InputProps } from "components/ui/Input";

type LabeledInputProps = Pick<ControlLabelsProps, "success" | "message" | "label" | "labelAdditionLength"> &
type LabeledInputProps = Pick<ControlLabelsProps, "success" | "message" | "label"> &
Copy link
Contributor Author

Choose a reason for hiding this comment

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

labelAdditionLength was not being used anywhere 🧹

InputProps & { className?: string };

const LabeledInput: React.FC<LabeledInputProps> = ({
error,
success,
message,
label,
labelAdditionLength,
className,
...inputProps
}) => (
<ControlLabels
error={error}
success={success}
message={message}
label={label}
className={className}
labelAdditionLength={labelAdditionLength}
>
const LabeledInput: React.FC<LabeledInputProps> = ({ error, success, message, label, className, ...inputProps }) => (
<ControlLabels error={error} success={success} message={message} label={label} className={className}>
<Input {...inputProps} error={error} />
</ControlLabels>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
.wrapper {
display: flex;
align-items: center;
justify-content: space-between;
}

.dropdownWrapper {
display: flex;
flex: 1 0 310px;
}

.spinner {
width: 50px;
display: flex;
justify-content: center;
align-items: center;
}

.dropdown {
flex-grow: 1;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import React, { useState } from "react";
import { FormattedMessage, useIntl } from "react-intl";

import { ControlLabels } from "components/LabeledControl";
import { Card } from "components/ui/Card";
import { DropDown } from "components/ui/DropDown";
import { Spinner } from "components/ui/Spinner";

import { Geography } from "core/request/AirbyteClient";
import { useConnectionEditService } from "hooks/services/ConnectionEdit/ConnectionEditService";
import { useNotificationService } from "hooks/services/Notification";
import { useAvailableGeographies } from "packages/cloud/services/geographies/GeographiesService";
import { links } from "utils/links";

import styles from "./UpdateConnectionDataResidency.module.scss";

export const UpdateConnectionDataResidency: React.FC = () => {
const { connection, updateConnection, connectionUpdating } = useConnectionEditService();
const { registerNotification } = useNotificationService();
const { formatMessage } = useIntl();
const [selectedValue, setSelectedValue] = useState<Geography>();

const { geographies } = useAvailableGeographies();

const handleSubmit = async ({ value }: { value: Geography }) => {
try {
setSelectedValue(value);
await updateConnection({
connectionId: connection.connectionId,
geography: value,
});
} catch (e) {
registerNotification({
id: "connection.geographyUpdateError",
title: formatMessage({ id: "connection.geographyUpdateError" }),
isError: true,
});
}
setSelectedValue(undefined);
};

return (
<Card withPadding>
<div className={styles.wrapper}>
<div>
<ControlLabels
nextLine
label={<FormattedMessage id="connection.geographyTitle" />}
message={
<FormattedMessage
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worth to point here out, when this will take effect? I assume it's not cancelling the current running sync and I also assume it doesn't require a reset, so simply the next sync/reset jobs will be done in that region? Maybe worth putting something like: "This will take effect for all syncs and resets starting after the change." (or something better worded).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is almost self-explanatory and not necessary to explain, but maybe @andyjih can weigh in. It's also something we could consider adding to the docs instead of directly in the UI.

Here's a proposal for the wording if we do want to mention it:

Depending on your network configuration, you may need to <lnk>add IP addresses</lnk> to your allowlist. Changes to data residency will be applied to new sync and reset jobs. Currently running jobs will not be affected.

id="connection.geographyDescription"
values={{
lnk: (node: React.ReactNode) => (
<a href={links.cloudAllowlistIPsLink} target="_blank" rel="noreferrer">
{node}
</a>
),
}}
/>
}
/>
</div>
<div className={styles.dropdownWrapper}>
<div className={styles.spinner}>{connectionUpdating && <Spinner small />}</div>
josephkmh marked this conversation as resolved.
Show resolved Hide resolved
<div className={styles.dropdown}>
<DropDown
isDisabled={connectionUpdating}
options={geographies.map((geography) => ({
label: formatMessage({ id: `connection.geography.${geography}` }),
josephkmh marked this conversation as resolved.
Show resolved Hide resolved
value: geography,
}))}
value={selectedValue || connection.geography}
onChange={handleSubmit}
/>
</div>
</div>
</div>
</Card>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { UpdateConnectionDataResidency } from "./UpdateConnectionDataResidency";
8 changes: 4 additions & 4 deletions airbyte-webapp/src/components/ui/Card/Card.module.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@use "../../../scss/colors";
@use "../../../scss/variables" as vars;
josephkmh marked this conversation as resolved.
Show resolved Hide resolved
@use "scss/colors";
@use "scss/variables";

.container {
width: auto;
Expand All @@ -17,9 +17,9 @@
}

.title {
padding: 25px 25px 22px;
padding: variables.$spacing-xl;
color: colors.$dark-blue;
border-bottom: colors.$grey-100 vars.$border-thin solid;
border-bottom: colors.$grey-100 variables.$border-thin solid;
font-weight: 600;
letter-spacing: 0.008em;
border-top-left-radius: 10px;
Expand Down
2 changes: 1 addition & 1 deletion airbyte-webapp/src/components/ui/SideMenu/SideMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ interface SideMenuProps {
}

const Content = styled.nav`
min-width: 147px;
min-width: 155px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The sidebar is ever-so-slightly too small for Default Data Residency. This CSS should really be more flexible, but just making it work for now!

`;

const Category = styled.div`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export enum PageTrackingCodes {
SETTINGS_NOTIFICATION = "Settings.Notifications",
SETTINGS_ACCESS_MANAGEMENT = "Settings.AccessManagement",
SETTINGS_METRICS = "Settings.Metrics",
SETTINGS_DATA_RESIDENCY = "Settings.DataResidency",
CREDITS = "Credits",
WORKSPACES = "Workspaces",
PREFERENCES = "Preferences",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { act, renderHook } from "@testing-library/react-hooks";
import React from "react";
import mockConnection from "test-utils/mock-data/mockConnection.json";
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 { WebBackendConnectionUpdate } from "core/request/AirbyteClient";
Expand All @@ -13,6 +14,10 @@ jest.mock("services/connector/DestinationDefinitionSpecificationService", () =>
useGetDestinationDefinitionSpecification: () => mockDest,
}));

jest.mock("services/workspaces/WorkspacesService", () => ({
useCurrentWorkspace: () => mockWorkspace,
}));

jest.mock("../useConnectionHook", () => ({
useGetConnection: () => mockConnection,
useWebConnectionService: () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { act, renderHook } from "@testing-library/react-hooks";
import React from "react";
import mockConnection from "test-utils/mock-data/mockConnection.json";
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 { AirbyteCatalog, WebBackendConnectionRead } from "core/request/AirbyteClient";
Expand All @@ -17,6 +18,10 @@ jest.mock("services/connector/DestinationDefinitionSpecificationService", () =>
useGetDestinationDefinitionSpecification: () => mockDest,
}));

jest.mock("services/workspaces/WorkspacesService", () => ({
useCurrentWorkspace: () => mockWorkspace,
}));

describe("ConnectionFormService", () => {
const Wrapper: React.FC<Parameters<typeof ConnectionFormServiceProvider>[0]> = ({ children, ...props }) => (
<TestWrapper>
Expand Down
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 @@ -5,6 +5,7 @@ export enum FeatureItem {
AllowUpdateConnectors = "ALLOW_UPDATE_CONNECTORS",
AllowOAuthConnector = "ALLOW_OAUTH_CONNECTOR",
AllowSync = "ALLOW_SYNC",
AllowChangeDataGeographies = "ALLOW_CHANGE_DATA_GEOGRAPHIES",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ability to change data geographies is behind a feature flag. It's not available in OSS, and it is currently disabled in cloud. We will do a staged rollout in production to test that it's working as expected.

AllowSyncSubOneHourCronExpressions = "ALLOW_SYNC_SUB_ONE_HOUR_CRON_EXPRESSIONS",
}

Expand Down
Loading