Skip to content

Commit

Permalink
🪟 🧹 Reorganize connections pages (#20845)
Browse files Browse the repository at this point in the history
* Reorganize connection pages
Flatten structure
Simplify names

* Consolidate all the Connection routes to a single file

* Extract job types and utilities to own files within JobItem
Move JobsWithJobs to JobItem/utils

* Move ConnectionName component to components/connection

* Move StatusMainInfo components and rename to components/connection/ConnectionInfoCard

* Move ConnectionBlock to components/connection

* Clean up ConnectionPage structure

* Update style import in ConnectionInfoCard test

* Clean up ConnectionRoutePaths enum

* Apply suggestions from code review - Update export default style

Co-authored-by: Krishna (kc) Glick <krishna@airbyte.io>

* Update ConnectionInfoCard test description name

* Update test snapshots

Co-authored-by: Krishna (kc) Glick <krishna@airbyte.io>
  • Loading branch information
2 people authored and jbfbell committed Jan 13, 2023
1 parent c97685c commit 45c2f20
Show file tree
Hide file tree
Showing 76 changed files with 336 additions and 305 deletions.
3 changes: 0 additions & 3 deletions airbyte-webapp/src/components/ConnectionBlock/index.tsx

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ exports[`CreateConnectionForm should render 1`] = `
<label
class="<removed-for-snapshot-test>"
>
Connection name*
Connection name
<span
class="<removed-for-snapshot-test>"
>
Expand Down Expand Up @@ -104,7 +104,7 @@ exports[`CreateConnectionForm should render 1`] = `
<label
class="<removed-for-snapshot-test>"
>
Replication frequency*
Replication frequency
<span
class="<removed-for-snapshot-test>"
>
Expand Down Expand Up @@ -243,7 +243,7 @@ exports[`CreateConnectionForm should render 1`] = `
<label
class="<removed-for-snapshot-test>"
>
Destination Namespace*
Destination Namespace
<span
class="<removed-for-snapshot-test>"
>
Expand Down Expand Up @@ -366,7 +366,7 @@ exports[`CreateConnectionForm should render 1`] = `
<label
class="<removed-for-snapshot-test>"
>
Destination Stream Prefix (Optional)
Destination Stream Prefix
<span
class="<removed-for-snapshot-test>"
>
Expand All @@ -390,6 +390,11 @@ exports[`CreateConnectionForm should render 1`] = `
</span>
</span>
</span>
<p
class="<removed-for-snapshot-test>"
>
Optional
</p>
</label>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ import styled from "styled-components";
import { Link } from "components";

import { useCurrentWorkspace } from "hooks/services/useWorkspace";
import { ConnectionSettingsRoutes } from "pages/ConnectionPage/pages/ConnectionItemPage/ConnectionSettingsRoutes";

import { RoutePaths } from "../../../pages/routePaths";
import { ConnectionRoutePaths } from "pages/connections/types";
import { RoutePaths } from "pages/routePaths";

interface IProps {
id: string;
Expand Down Expand Up @@ -39,7 +38,7 @@ const ConnectorCell: React.FC<IProps> = ({ id }) => {
event.stopPropagation();
};

const settingPath = `/${RoutePaths.Workspaces}/${workspaceId}/${RoutePaths.Connections}/${id}/${ConnectionSettingsRoutes.REPLICATION}`;
const settingPath = `/${RoutePaths.Workspaces}/${workspaceId}/${RoutePaths.Connections}/${id}/${ConnectionRoutePaths.Replication}`;
return (
<Content onClick={openSettings}>
<Link to={settingPath}>
Expand Down
21 changes: 5 additions & 16 deletions airbyte-webapp/src/components/JobItem/JobItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ import styled from "styled-components";

import { Spinner } from "components/ui/Spinner";

import { JobsWithJobs } from "pages/ConnectionPage/pages/ConnectionItemPage/JobsList";
import { SynchronousJobRead } from "core/request/AirbyteClient";

import { AttemptRead, JobStatus, SynchronousJobRead } from "../../core/request/AirbyteClient";
import { useAttemptLink } from "./attemptLinkUtils";
import ContentWrapper from "./components/ContentWrapper";
import ErrorDetails from "./components/ErrorDetails";
import { JobLogs } from "./components/JobLogs";
import MainInfo from "./components/MainInfo";
import styles from "./JobItem.module.scss";
import { JobsWithJobs } from "./types";
import { didJobSucceed, getJobAttempts, getJobId } from "./utils";

const Item = styled.div<{ isFailed: boolean }>`
border-bottom: 1px solid ${({ theme }) => theme.greyColor20};
Expand All @@ -27,18 +28,6 @@ interface JobItemProps {
job: SynchronousJobRead | JobsWithJobs;
}

const didJobSucceed = (job: SynchronousJobRead | JobsWithJobs): boolean =>
"succeeded" in job ? job.succeeded : getJobStatus(job) !== "failed";

export const getJobStatus: (job: SynchronousJobRead | JobsWithJobs) => JobStatus = (job) =>
"succeeded" in job ? (job.succeeded ? JobStatus.succeeded : JobStatus.failed) : job.job.status;

export const getJobAttemps: (job: SynchronousJobRead | JobsWithJobs) => AttemptRead[] | undefined = (job) =>
"attempts" in job ? job.attempts : undefined;

export const getJobId = (job: SynchronousJobRead | JobsWithJobs): string | number =>
"id" in job ? job.id : job.job.id;

export const JobItem: React.FC<JobItemProps> = ({ job }) => {
const { jobId: linkedJobId } = useAttemptLink();
const alreadyScrolled = useRef(false);
Expand All @@ -63,7 +52,7 @@ export const JobItem: React.FC<JobItemProps> = ({ job }) => {

return (
<Item isFailed={!didSucceed} ref={scrollAnchor}>
<MainInfo isOpen={isOpen} isFailed={!didSucceed} onExpand={onExpand} job={job} attempts={getJobAttemps(job)} />
<MainInfo isOpen={isOpen} isFailed={!didSucceed} onExpand={onExpand} job={job} attempts={getJobAttempts(job)} />
<ContentWrapper isOpen={isOpen} onToggled={onDetailsToggled}>
<div>
<Suspense
Expand All @@ -75,7 +64,7 @@ export const JobItem: React.FC<JobItemProps> = ({ job }) => {
>
{isOpen && (
<>
<ErrorDetails attempts={getJobAttemps(job)} />
<ErrorDetails attempts={getJobAttempts(job)} />
<JobLogs job={job} jobIsFailed={!didSucceed} />
</>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import { StatusIconStatus } from "components/ui/StatusIcon/StatusIcon";
import { Text } from "components/ui/Text";

import { AttemptRead, AttemptStatus, SynchronousJobRead } from "core/request/AirbyteClient";
import { JobsWithJobs } from "pages/ConnectionPage/pages/ConnectionItemPage/JobsList";
import { useGetDebugInfoJob } from "services/job/JobService";

import { parseAttemptLink } from "../attemptLinkUtils";
import { JobsWithJobs } from "../types";
import { isCancelledAttempt } from "../utils";
import styles from "./JobLogs.module.scss";
import Logs from "./Logs";
Expand Down
4 changes: 2 additions & 2 deletions airbyte-webapp/src/components/JobItem/components/MainInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import { Cell, Row } from "components/SimpleTableComponents";
import { StatusIcon } from "components/ui/StatusIcon";

import { AttemptRead, JobStatus, SynchronousJobRead } from "core/request/AirbyteClient";
import { JobsWithJobs } from "pages/ConnectionPage/pages/ConnectionItemPage/JobsList";

import { getJobStatus } from "../JobItem";
import { JobsWithJobs } from "../types";
import { getJobStatus } from "../utils";
import { AttemptDetails } from "./AttemptDetails";
import styles from "./MainInfo.module.scss";
import { ResetStreamsDetails } from "./ResetStreamDetails";
Expand Down
1 change: 1 addition & 0 deletions airbyte-webapp/src/components/JobItem/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./JobItem";
5 changes: 5 additions & 0 deletions airbyte-webapp/src/components/JobItem/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { JobWithAttemptsRead } from "core/request/AirbyteClient";

export interface JobsWithJobs extends JobWithAttemptsRead {
job: Exclude<JobWithAttemptsRead["job"], undefined>;
}
22 changes: 21 additions & 1 deletion airbyte-webapp/src/components/JobItem/utils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
import { AttemptFailureReason, AttemptFailureType, AttemptRead } from "core/request/AirbyteClient";
import {
AttemptFailureReason,
AttemptFailureType,
AttemptRead,
JobStatus,
SynchronousJobRead,
} from "core/request/AirbyteClient";

import { JobsWithJobs } from "./types";

export const getFailureFromAttempt = (attempt: AttemptRead): AttemptFailureReason | undefined =>
attempt.failureSummary?.failures[0];

export const isCancelledAttempt = (attempt: AttemptRead): boolean =>
attempt.failureSummary?.failures.some(({ failureType }) => failureType === AttemptFailureType.manual_cancellation) ??
false;

export const didJobSucceed = (job: SynchronousJobRead | JobsWithJobs): boolean =>
"succeeded" in job ? job.succeeded : getJobStatus(job) !== "failed";

export const getJobStatus: (job: SynchronousJobRead | JobsWithJobs) => JobStatus = (job) =>
"succeeded" in job ? (job.succeeded ? JobStatus.succeeded : JobStatus.failed) : job.job.status;

export const getJobAttempts: (job: SynchronousJobRead | JobsWithJobs) => AttemptRead[] | undefined = (job) =>
"attempts" in job ? job.attempts : undefined;

export const getJobId = (job: SynchronousJobRead | JobsWithJobs): string | number =>
"id" in job ? job.id : job.job.id;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@use "../../scss/colors";
@use "scss/colors";

.lightContentCard {
display: flex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ interface IProps {
itemTo?: { name: string; icon?: string };
}

const ConnectionBlock: React.FC<IProps> = (props) => (
export const ConnectionBlock: React.FC<IProps> = (props) => (
<Card className={classNames(styles.lightContentCard)}>
{props.itemFrom ? <ConnectionBlockItem {...props.itemFrom} /> : <Content className={styles.extraBlock} />}
<FontAwesomeIcon className={styles.arrow} icon={faChevronRight} />
{props.itemTo ? <ConnectionBlockItem {...props.itemTo} /> : <Content className={styles.extraBlock} />}
</Card>
);

export default ConnectionBlock;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./ConnectionBlock";
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { ConnectionStatus, SchemaChange } from "core/request/AirbyteClient";
import { defaultOssFeatures, FeatureItem } from "hooks/services/Feature";

// eslint-disable-next-line css-modules/no-unused-class
import styles from "./StatusMainInfo.module.scss";
import styles from "./ConnectionInfoCard.module.scss";

const mockUseConnectionEditService = jest.fn();

Expand All @@ -32,9 +32,9 @@ const TestWrapperWithAutoDetectSchema: React.FC<React.PropsWithChildren<Record<s
);

// eslint-disable-next-line @typescript-eslint/no-var-requires
const { StatusMainInfo } = require("./StatusMainInfo");
const { ConnectionInfoCard } = require("./ConnectionInfoCard");

describe("<StatusMainInfo />", () => {
describe(`<${ConnectionInfoCard.name} />`, () => {
beforeEach(() => {
mockUseConnectionEditService.mockReturnValue({
connection: mockConnection,
Expand All @@ -43,16 +43,16 @@ describe("<StatusMainInfo />", () => {
});

it("renders", () => {
const { getByTestId, queryByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapperWithAutoDetectSchema });
const { getByTestId, queryByTestId } = render(<ConnectionInfoCard />, { wrapper: TestWrapperWithAutoDetectSchema });

expect(getByTestId("statusMainInfo")).toBeDefined();
expect(getByTestId("connectionInfo")).toBeDefined();

expect(getByTestId("enabledControl")).toBeDefined();
expect(getByTestId("enabledControl-switch")).toBeEnabled();

// schema changes-related
expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.breaking);
expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.nonBreaking);
expect(getByTestId("connectionInfo-sourceLink")).not.toHaveClass(styles.breaking);
expect(getByTestId("connectionInfo-sourceLink")).not.toHaveClass(styles.nonBreaking);
expect(queryByTestId("schemaChangesDetected")).toBeFalsy();
});

Expand All @@ -62,10 +62,10 @@ describe("<StatusMainInfo />", () => {
schemaHasBeenRefreshed: false,
});

const { getByTestId, queryByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapperWithAutoDetectSchema });
const { getByTestId, queryByTestId } = render(<ConnectionInfoCard />, { wrapper: TestWrapperWithAutoDetectSchema });

expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.breaking);
expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.nonBreaking);
expect(getByTestId("connectionInfo-sourceLink")).not.toHaveClass(styles.breaking);
expect(getByTestId("connectionInfo-sourceLink")).not.toHaveClass(styles.nonBreaking);

expect(queryByTestId("enabledControl")).toBeFalsy();
expect(queryByTestId("schemaChangesDetected")).toBeFalsy();
Expand All @@ -77,10 +77,10 @@ describe("<StatusMainInfo />", () => {
schemaHasBeenRefreshed: false,
});

const { getByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapperWithAutoDetectSchema });
const { getByTestId } = render(<ConnectionInfoCard />, { wrapper: TestWrapperWithAutoDetectSchema });

expect(getByTestId("statusMainInfo-sourceLink")).toHaveClass(styles.breaking);
expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.nonBreaking);
expect(getByTestId("connectionInfo-sourceLink")).toHaveClass(styles.breaking);
expect(getByTestId("connectionInfo-sourceLink")).not.toHaveClass(styles.nonBreaking);
expect(getByTestId("schemaChangesDetected")).toBeDefined();

expect(getByTestId("enabledControl-switch")).toBeDisabled();
Expand All @@ -92,10 +92,10 @@ describe("<StatusMainInfo />", () => {
schemaHasBeenRefreshed: false,
});

const { getByTestId } = render(<StatusMainInfo />, { wrapper: TestWrapperWithAutoDetectSchema });
const { getByTestId } = render(<ConnectionInfoCard />, { wrapper: TestWrapperWithAutoDetectSchema });

expect(getByTestId("statusMainInfo-sourceLink")).not.toHaveClass(styles.breaking);
expect(getByTestId("statusMainInfo-sourceLink")).toHaveClass(styles.nonBreaking);
expect(getByTestId("connectionInfo-sourceLink")).not.toHaveClass(styles.breaking);
expect(getByTestId("connectionInfo-sourceLink")).toHaveClass(styles.nonBreaking);
expect(getByTestId("schemaChangesDetected")).toBeDefined();

expect(getByTestId("enabledControl-switch")).toBeEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import { RoutePaths } from "pages/routePaths";
import { useDestinationDefinition } from "services/connector/DestinationDefinitionService";
import { useSourceDefinition } from "services/connector/SourceDefinitionService";

import styles from "./ConnectionInfoCard.module.scss";
import { EnabledControl } from "./EnabledControl";
import { SchemaChangesDetected } from "./SchemaChangesDetected";
import styles from "./StatusMainInfo.module.scss";

export const StatusMainInfo: React.FC = () => {
export const ConnectionInfoCard: React.FC = () => {
const {
connection: { source, destination, status, schemaChange },
schemaHasBeenRefreshed,
Expand All @@ -43,12 +43,12 @@ export const StatusMainInfo: React.FC = () => {
};

return (
<div className={styles.container} data-testid="statusMainInfo">
<div className={styles.container} data-testid="connectionInfo">
<div className={styles.pathContainer}>
<Link
to={sourceConnectionPath}
className={classNames(styles.connectorLink, schemaChangeClassNames)}
data-testid="statusMainInfo-sourceLink"
data-testid="connectionInfo-sourceLink"
>
<ConnectorCard
connectionName={source.sourceName}
Expand All @@ -61,7 +61,7 @@ export const StatusMainInfo: React.FC = () => {
<Link
to={destinationConnectionPath}
className={styles.connectorLink}
data-testid="statusMainInfo-destinationLink"
data-testid="connectionInfo-destinationLink"
>
<ConnectorCard
connectionName={destination.destinationName}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import { Text } from "components/ui/Text";
import { useSchemaChanges } from "hooks/connection/useSchemaChanges";
import { useConnectionEditService } from "hooks/services/ConnectionEdit/ConnectionEditService";
import { useFormChangeTrackerService } from "hooks/services/FormChangeTracker";
import { ConnectionRoutePaths } from "pages/connections/types";
import { useRefreshSourceSchemaWithConfirmationOnDirty } from "views/Connection/ConnectionForm/components/refreshSourceSchemaWithConfirmationOnDirty";

import { ConnectionSettingsRoutes } from "./ConnectionSettingsRoutes";
import styles from "./SchemaChangesDetected.module.scss";

export const SchemaChangesDetected: React.FC = () => {
Expand All @@ -31,8 +31,8 @@ export const SchemaChangesDetected: React.FC = () => {
}

const onReviewActionButtonClick: React.MouseEventHandler<HTMLButtonElement> = () => {
if (!location.pathname.includes(`/${ConnectionSettingsRoutes.REPLICATION}`)) {
navigate(ConnectionSettingsRoutes.REPLICATION);
if (!location.pathname.includes(`/${ConnectionRoutePaths.Replication}`)) {
navigate(ConnectionRoutePaths.Replication);
}

refreshSchema();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./ConnectionInfoCard";
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@use "../../../../scss/colors";
@use "../../../../scss/variables";
@use "scss/colors";
@use "scss/variables";

.container {
margin-top: variables.$spacing-md;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./ConnectionName";
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import classNames from "classnames";
import { FormattedMessage, useIntl } from "react-intl";

import { getJobStatus } from "components/JobItem/JobItem";
import { JobsWithJobs } from "components/JobItem/types";
import { getJobStatus } from "components/JobItem/utils";
import { Text } from "components/ui/Text";

import { AttemptRead, AttemptStatus, SynchronousJobRead } from "core/request/AirbyteClient";
import { JobsWithJobs } from "pages/ConnectionPage/pages/ConnectionItemPage/JobsList";
import { formatBytes } from "utils/numberHelper";

import styles from "./JobProgress.module.scss";
Expand Down
Loading

0 comments on commit 45c2f20

Please sign in to comment.