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

Highlight invalid cursor or primary key in new connection streams table #20756

Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,12 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({
);

const destName = prefix + (streamNode.stream?.name ?? "");
const configErrors = getIn(errors, `schema.streams[${streamNode.id}].config`);
const configErrors = getIn(
errors,
isNewStreamsTableEnabled
? `syncCatalog.streams[${streamNode.id}].config`
: `schema.streams[${streamNode.id}].config`
edmundito marked this conversation as resolved.
Show resolved Hide resolved
);
const hasError = configErrors && Object.keys(configErrors).length > 0;
const pkType = getPathType(pkRequired, shouldDefinePk);
const cursorType = getPathType(cursorRequired, shouldDefineCursor);
Expand Down Expand Up @@ -194,6 +199,7 @@ const CatalogSectionInner: React.FC<CatalogSectionInnerProps> = ({
onExpand={onExpand}
changedSelected={changedSelected}
hasError={hasError}
configErrors={configErrors}
disabled={disabled}
/>
{isRowExpanded &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface StreamHeaderProps {
onExpand: () => void;
changedSelected: boolean;
hasError: boolean;
configErrors?: Record<string, string>;
disabled?: boolean;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@
@include header-background(colors.$blue-30, colors.$blue-40);
}

&.error {
border: 1px solid colors.$red;
}

&.disabled {
background-color: colors.$grey-50;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ export const CatalogTreeTableRow: React.FC<StreamHeaderProps> = ({
fields,
onExpand,
disabled,
configErrors,
}) => {
const { primaryKey, cursorField, syncMode, destinationSyncMode } = stream.config ?? {};

const { defaultCursorField } = stream.stream ?? {};
const syncSchema = useMemo(
() => ({
Expand Down Expand Up @@ -104,6 +104,7 @@ export const CatalogTreeTableRow: React.FC<StreamHeaderProps> = ({
path={cursorType === "sourceDefined" ? defaultCursorField : cursorField}
onPathChange={onCursorChange}
variant={pillButtonVariant}
hasError={!!configErrors?.cursorField}
/>
)}
</Cell>
Expand All @@ -116,6 +117,7 @@ export const CatalogTreeTableRow: React.FC<StreamHeaderProps> = ({
isMulti
onPathChange={onPrimaryKeyChange}
variant={pillButtonVariant}
hasError={!!configErrors?.primaryKey}
/>
)}
</Cell>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface StreamPathSelectBaseProps {
placeholder?: React.ReactNode;
variant?: PillButtonVariant;
disabled?: boolean;
hasError?: boolean;
}

interface StreamPathSelectMultiProps {
Expand Down Expand Up @@ -69,6 +70,7 @@ export const StreamPathSelect: React.FC<PathPopoutProps> = (props) => {
props.onPathChange(finalValues);
}}
className={styles.streamPathSelect}
hasError={props?.hasError}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,4 @@ describe("useCatalogTreeTableRowProps", () => {
expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent changed");
expect(result.current.pillButtonVariant).toEqual("blue");
});
it("should return error styles for a row that has an error", () => {
testSetup(mockInitialValues, false, "error");

const { result } = renderHook(() => useCatalogTreeTableRowProps(mockStream));

expect(result.current.streamHeaderContentStyle).toEqual("streamHeaderContent error");
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable css-modules/no-unused-class */
import classNames from "classnames";
import { useField } from "formik";
import isEqual from "lodash/isEqual";
import { useMemo } from "react";

Expand All @@ -11,16 +10,12 @@ import { useBulkEditSelect } from "hooks/services/BulkEdit/BulkEditService";
import { useConnectionFormService } from "hooks/services/ConnectionForm/ConnectionFormService";

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

type StatusToDisplay = "disabled" | "added" | "removed" | "changed" | "unchanged";

export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => {
const { initialValues } = useConnectionFormService();
const [isSelected] = useBulkEditSelect(stream.id);

const [, { error }] = useField(`schema.streams[${stream.id}].config`);

// in case error is an empty string
const hasError = error !== undefined;
const { initialValues } = useConnectionFormService();

const isStreamEnabled = stream.config?.selected;

Expand Down Expand Up @@ -67,7 +62,6 @@ export const useCatalogTreeTableRowProps = (stream: SyncSchemaStream) => {
[styles.removed]: statusToDisplay === "removed" && !isSelected,
[styles.changed]: statusToDisplay === "changed" || isSelected,
[styles.disabled]: statusToDisplay === "disabled",
[styles.error]: hasError,
});

return {
Expand Down
12 changes: 10 additions & 2 deletions airbyte-webapp/src/components/ui/PillSelect/PillButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,27 @@ const STYLES_BY_VARIANT: Readonly<Record<PillButtonVariant, string>> = {
interface PillButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
active?: boolean;
variant?: PillButtonVariant;
hasError?: boolean;
}

export const PillButton: React.FC<PillButtonProps> = ({ children, active, variant = "grey", ...buttonProps }) => {
export const PillButton: React.FC<PillButtonProps> = ({
children,
active,
variant = "grey",
hasError = false,
...buttonProps
}) => {
const buttonClassName = classNames(
styles.button,
{
[styles.active]: active,
[styles.disabled]: buttonProps.disabled,
},
STYLES_BY_VARIANT[variant],
STYLES_BY_VARIANT[hasError ? "strong-red" : variant],
buttonProps.className
);
const arrayChildren = Children.toArray(children);

return (
<button type="button" {...buttonProps} className={buttonClassName}>
{Children.map(arrayChildren, (child, index) => (
Expand Down
2 changes: 2 additions & 0 deletions airbyte-webapp/src/components/ui/PillSelect/PillSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ type PickedPopoutProps = Pick<PopoutProps, "value" | "options" | "isMulti" | "on
interface PillSelectProps extends PickedPopoutProps {
variant?: PillButtonVariant;
disabled?: boolean;
hasError?: boolean;
}

export const PillSelect: React.FC<PillSelectProps> = ({ className, ...props }) => {
Expand All @@ -33,6 +34,7 @@ export const PillSelect: React.FC<PillSelectProps> = ({ className, ...props }) =
}}
active={isOpen}
className={className}
hasError={props?.hasError}
>
{label}
</PillButton>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,23 @@ const useConnectionForm = ({
const formId = useUniqueFormId();

const getErrorMessage = useCallback(
(formValid: boolean, connectionDirty: boolean) =>
submitError
(formValid: boolean, connectionDirty: boolean) => {
const isNewStreamsTableEnabled = process.env.REACT_APP_NEW_STREAMS_TABLE ?? false;

if (isNewStreamsTableEnabled) {
// There is a case when some fields could be dropped in the database. We need to validate the form without property dirty
return submitError
? generateMessageFromError(submitError)
: !formValid
? formatMessage({ id: "connectionForm.validation.error" })
: null;
}
return submitError
? generateMessageFromError(submitError)
: connectionDirty && !formValid
? formatMessage({ id: "connectionForm.validation.error" })
: null,
: null;
},
[formatMessage, submitError]
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,18 +203,20 @@ export const ConnectionReplicationTab: React.FC = () => {
dirty={dirty || schemaHasBeenRefreshed}
/>
{status.editControlsVisible && (
<EditControls
isSubmitting={isSubmitting}
submitDisabled={!isValid}
dirty={dirty}
resetForm={async () => {
resetForm();
discardRefreshedSchema();
}}
successMessage={saved && !dirty && <FormattedMessage id="form.changesSaved" />}
errorMessage={getErrorMessage(isValid, dirty)}
enableControls={schemaHasBeenRefreshed || dirty}
/>
<div>
edmundito marked this conversation as resolved.
Show resolved Hide resolved
<EditControls
isSubmitting={isSubmitting}
submitDisabled={!isValid}
dirty={dirty}
resetForm={async () => {
resetForm();
discardRefreshedSchema();
}}
successMessage={saved && !dirty && <FormattedMessage id="form.changesSaved" />}
errorMessage={getErrorMessage(isValid, dirty)}
enableControls={schemaHasBeenRefreshed || dirty}
/>
</div>
)}
</Form>
</SchemaChangeBackdrop>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -760,34 +760,36 @@ exports[`ConnectionReplicationTab should render 1`] = `
</div>
</div>
</div>
<div
class="<removed-for-snapshot-test>"
>
<div>
<div
class="<removed-for-snapshot-test>"
>
<button
<div
class="<removed-for-snapshot-test>"
disabled=""
type="button"
>
<div
<button
class="<removed-for-snapshot-test>"
disabled=""
type="button"
>
Cancel
</div>
</button>
<button
class="<removed-for-snapshot-test>"
disabled=""
type="submit"
>
<div
<div
class="<removed-for-snapshot-test>"
>
Cancel
</div>
</button>
<button
class="<removed-for-snapshot-test>"
disabled=""
type="submit"
>
Save changes
</div>
</button>
<div
class="<removed-for-snapshot-test>"
>
Save changes
</div>
</button>
</div>
</div>
</div>
</form>
Expand Down