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

Force register a new schema #1877

Merged
merged 16 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 37 additions & 1 deletion coral/src/app/components/Form.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
RadioButtonGroup,
Textarea,
TextInput,
Checkbox,
} from "src/app/components/Form";
import {
renderForm,
Expand Down Expand Up @@ -47,7 +48,7 @@ describe("Form", () => {
};

const assertSubmitted = (
data: Record<string, string | number | string[]>
data: Record<string, string | number | string[] | boolean>
) => {
expect(onSubmit).toHaveBeenCalledWith(data, expect.anything());
};
Expand Down Expand Up @@ -929,4 +930,39 @@ describe("Form", () => {
expect(errorMessage).toBeVisible();
});
});

describe("<Checkbox>", () => {
const schema = z.object({
areYouForReal: z.boolean(),
});
type Schema = z.infer<typeof schema>;

beforeEach(() => {
results = renderForm(
<Checkbox<Schema> name={"areYouForReal"}>Are you sure?</Checkbox>,
{ schema, onSubmit, onError }
);
});

it("should render a Checkbox with correct label", () => {
const checkbox = screen.getByRole("checkbox", { name: "Are you sure?" });
expect(checkbox).toBeEnabled();
});

it("should default to Checkbox being unchecked when no default values are provided", async () => {
const checkbox = screen.getByRole("checkbox", { name: "Are you sure?" });
expect(checkbox).not.toBeChecked();
});

it("should sync value to form state when clicking Checkbox", async () => {
const checkbox = screen.getByRole("checkbox", { name: "Are you sure?" });
expect(checkbox).not.toBeChecked();

await user.click(checkbox);
expect(checkbox).toBeChecked();

await submit();
assertSubmitted({ areYouForReal: true });
});
});
});
23 changes: 23 additions & 0 deletions coral/src/app/components/Form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
RadioButtonProps as BaseRadioButtonProps,
Textarea as BaseTextarea,
TextareaProps as BaseTextareaProps,
Checkbox as BaseCheckbox,
CheckboxProps as BaseCheckboxProps,
Option,
OptionType,
Button,
Expand Down Expand Up @@ -699,3 +701,24 @@ export const FileInput = <T extends FieldValues>(
const ctx = useFormContext<T>();
return <FileInputMemo formContext={ctx} {...props} />;
};

// <Checkbox
function _Checkbox<T extends FieldValues>({
name,
formContext: form,
...props
}: BaseCheckboxProps & FormInputProps<T> & FormRegisterProps<T>) {
return <BaseCheckbox {...props} {...form.register(name)} />;
}

const CheckboxMemo = memo(_Checkbox) as typeof _Checkbox;

// eslint-disable-next-line import/exports-last,import/group-exports
export const Checkbox = <T extends FieldValues>(
props: FormInputProps<T> & BaseCheckboxProps
): React.ReactElement<FormInputProps<T> & BaseCheckboxProps> => {
const ctx = useFormContext<T>();
return <CheckboxMemo formContext={ctx} {...props} />;
};

TextInput.Skeleton = BaseInput.Skeleton;
Original file line number Diff line number Diff line change
Expand Up @@ -922,6 +922,179 @@ describe("TopicDetailsSchema", () => {
});
});
});

describe("enables topic owner to promote a schema even if it's not compatible", () => {
const originalConsoleError = console.error;

beforeEach(() => {
console.error = jest.fn();

mockedUseTopicDetails.mockReturnValue({
topicOverviewIsRefetching: false,
topicSchemasIsRefetching: false,
topicName: testTopicName,
environmentId: testEnvironmentId,
topicSchemas: testTopicSchemas,
setSchemaVersion: mockSetSchemaVersion,
topicOverview: { topicInfo: { topicOwner: true } },
});

customRender(
<AquariumContext>
<TopicDetailsSchema />
</AquariumContext>,
{
memoryRouter: true,
queryClient: true,
}
);
});

afterEach(() => {
console.error = originalConsoleError;
cleanup();
jest.clearAllMocks();
});

it("gives user option to force register if request fails with certain error", async () => {
// The first response to the test should be the compatibility error
// second response will be the success
mockPromoteSchemaRequest
.mockRejectedValueOnce({
success: false,
message: "failure: Schema is not compatible",
})
.mockResolvedValue({
success: true,
message: "",
});

const checkBoxBefore = screen.queryByRole("checkbox", {
name: "Force register Overrides standard validation processes of the schema registry.",
});

expect(checkBoxBefore).not.toBeInTheDocument();

const buttonPromote = screen.getByRole("button", { name: "Promote" });

await user.click(buttonPromote);

const modal = screen.getByRole("dialog");
const buttonRequest = within(modal).getByRole("button", {
name: "Request schema promotion",
});

await user.click(buttonRequest);

expect(mockPromoteSchemaRequest).toHaveBeenCalledWith({
forceRegister: false,
remarks: "",
schemaVersion: "3",
sourceEnvironment: "1",
targetEnvironment: "2",
topicName: "topic-name",
});

const checkboxToForceRegister = screen.getByRole("checkbox", {
name: "Force register Overrides standard validation processes of the schema registry.",
});

expect(checkboxToForceRegister).toBeVisible();

expect(console.error).toHaveBeenCalledWith({
message: "failure: Schema is not compatible",
success: false,
});
});

it("enables user to force register the schema if needed", async () => {
// The first response to the test should be the compatibility error
// second response will be the success
mockPromoteSchemaRequest
.mockRejectedValueOnce({
success: false,
message: "failure: Schema is not compatible",
})
.mockResolvedValue({
success: true,
message: "",
});

const buttonPromote = screen.getByRole("button", { name: "Promote" });

await user.click(buttonPromote);

const modal = screen.getByRole("dialog");
const buttonRequest = within(modal).getByRole("button", {
name: "Request schema promotion",
});

await user.click(buttonRequest);

const checkboxToForceRegister = screen.getByRole("checkbox", {
name: "Force register Overrides standard validation processes of the schema registry.",
});

await user.click(checkboxToForceRegister);
await user.click(buttonRequest);

expect(mockPromoteSchemaRequest).toHaveBeenNthCalledWith(2, {
forceRegister: true,
remarks: "",
schemaVersion: "3",
sourceEnvironment: "1",
targetEnvironment: "2",
topicName: "topic-name",
});

expect(console.error).toHaveBeenCalledWith({
message: "failure: Schema is not compatible",
success: false,
});
});

it("shows an error if promotion with force register did fail", async () => {
// The first response to the test should be the compatibility error
// second response will be the success
mockPromoteSchemaRequest
.mockRejectedValueOnce({
success: false,
message: "failure: Schema is not compatible",
})
.mockRejectedValue({
success: false,
message: "Oh no",
});

const buttonPromote = screen.getByRole("button", { name: "Promote" });

await user.click(buttonPromote);

const modal = screen.getByRole("dialog");
const buttonRequest = within(modal).getByRole("button", {
name: "Request schema promotion",
});

await user.click(buttonRequest);

const checkboxToForceRegister = screen.getByRole("checkbox", {
name: "Force register Overrides standard validation processes of the schema registry.",
});

await user.click(checkboxToForceRegister);
await user.click(buttonRequest);

const alert = screen.getByRole("alert");
const errorMessage = within(alert).getByText("Oh no");

expect(alert).toBeVisible();
expect(errorMessage).toBeVisible();
expect(console.error).toHaveBeenNthCalledWith(2, {
success: false,
message: "Oh no",
});
});
});
});

describe("renders right view for user that is not topic owner", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ function TopicDetailsSchema() {
const [showSchemaPromotionModal, setShowSchemaPromotionModal] =
useState(false);
const [errorMessage, setErrorMessage] = useState("");
const [isValidationError, setIsValidationError] = useState(false);

const toast = useToast();

Expand Down Expand Up @@ -88,10 +89,17 @@ function TopicDetailsSchema() {
},
{
onError: (error: HTTPError) => {
setErrorMessage(parseErrorMsg(error));
setShowSchemaPromotionModal(false);
const message = parseErrorMsg(error);
if (message.includes("Schema is not compatible")) {
setIsValidationError(true);
} else {
setErrorMessage(message);
setShowSchemaPromotionModal(false);
setIsValidationError(false);
}
},
onSuccess: () => {
setIsValidationError(false);
setErrorMessage("");
queryClient.refetchQueries(["schema-overview"]).then(() => {
setShowSchemaPromotionModal(false);
Expand Down Expand Up @@ -132,10 +140,7 @@ function TopicDetailsSchema() {
version={schemaDetailsPerEnv.version}
// We only allow users to use the forceRegister option when the promotion request failed
// And the failure is because of a schema compatibility issue
showForceRegister={
errorMessage.length > 0 &&
errorMessage.includes("Schema is not compatible")
}
showForceRegister={isValidationError}
/>
)}
<PageHeader title="Schema" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ describe("SchemaPromotionModal", () => {
expect(dialog).toBeVisible();
});

it("shows no warning or option to force register", () => {
const warning = screen.queryByRole("alert");
const checkbox = screen.queryByRole("checkbox");

expect(warning).not.toBeInTheDocument();
expect(checkbox).not.toBeInTheDocument();
});

it("shows more information to delete the topic", () => {
const dialog = screen.getByRole("dialog");
const headline = within(dialog).getByRole("heading", {
Expand All @@ -50,7 +58,7 @@ describe("SchemaPromotionModal", () => {

it("does not show a switch to force register", () => {
const forceRegisterSwitch = screen.queryByRole("checkbox", {
name: "Force register Overrides some validation that the schema registry would normally do.",
name: "Force register Overrides standard validation processes of the schema registry.",
});
expect(forceRegisterSwitch).not.toBeInTheDocument();
});
Expand Down Expand Up @@ -102,13 +110,6 @@ describe("SchemaPromotionModal", () => {
jest.clearAllMocks();
});

it("disables Force register switch", () => {
const forceRegisterSwitch = screen.getByRole("checkbox", {
name: "Force register Overrides some validation that the schema registry would normally do.",
});
expect(forceRegisterSwitch).toBeDisabled();
});

it("disables textarea where user can add a comment why they promote the schema", () => {
const dialog = screen.getByRole("dialog");
const textarea = within(dialog).getByRole("textbox", {
Expand Down Expand Up @@ -191,6 +192,15 @@ describe("SchemaPromotionModal", () => {
jest.clearAllMocks();
});

it("shows a warning about force register", async () => {
const warning = screen.getByRole("alert");

expect(warning).toBeVisible();
expect(warning).toHaveTextContent(
"Uploaded schema appears invalid. Are you sure you want to force register it?"
);
});

it("triggers a given submit function with correct payload when user does not switch Force register or adds a reason", async () => {
const dialog = screen.getByRole("dialog");

Expand All @@ -211,7 +221,7 @@ describe("SchemaPromotionModal", () => {
const dialog = screen.getByRole("dialog");

const forceRegisterSwitch = screen.getByRole("checkbox", {
name: "Force register Overrides some validation that the schema registry would normally do.",
name: "Force register Overrides standard validation processes of the schema registry.",
});

const confirmationButton = within(dialog).getByRole("button", {
Expand All @@ -232,7 +242,7 @@ describe("SchemaPromotionModal", () => {
const dialog = screen.getByRole("dialog");

const forceRegisterSwitch = screen.getByRole("checkbox", {
name: "Force register Overrides some validation that the schema registry would normally do.",
name: "Force register Overrides standard validation processes of the schema registry.",
});

const textarea = within(dialog).getByRole("textbox", {
Expand Down
Loading