From f173c80df3c5606282a89fbe953137bba116ada3 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Mon, 5 Dec 2022 23:47:55 +0200
Subject: [PATCH] Migrate admin to the new database form (#26923)
---
frontend/src/metabase-types/store/setup.ts | 4 +-
.../databases/containers/DatabaseEditApp.jsx | 69 +++----------------
.../containers/DatabaseEditApp.unit.spec.js | 2 +
.../FormFileInput/FormFileInput.tsx | 1 -
.../DatabaseCacheScheduleField.tsx | 39 +++++------
.../DatabaseDetailField.tsx | 3 +-
.../DatabaseEngineField.tsx | 4 +-
.../components/DatabaseForm/DatabaseForm.tsx | 51 ++++++++------
.../DatabaseScheduleToggleField.tsx | 14 ++--
frontend/src/metabase/databases/types.ts | 4 +-
.../src/metabase/databases/utils/engine.ts | 49 ++++++++-----
.../src/metabase/databases/utils/schema.ts | 58 ++++++++++++----
.../admin/databases/add-external.cy.spec.js | 25 ++++---
.../admin/databases/add-presto.cy.spec.js | 14 ++--
.../scenarios/admin/databases/add.cy.spec.js | 37 +---------
.../scenarios/admin/databases/edit.cy.spec.js | 25 +++----
16 files changed, 185 insertions(+), 214 deletions(-)
diff --git a/frontend/src/metabase-types/store/setup.ts b/frontend/src/metabase-types/store/setup.ts
index 51b4ac9a093..60cc4ad8695 100644
--- a/frontend/src/metabase-types/store/setup.ts
+++ b/frontend/src/metabase-types/store/setup.ts
@@ -25,8 +25,8 @@ export interface DatabaseInfo {
engine: string | undefined;
details: Record<string, unknown>;
schedules: DatabaseSchedules;
- auto_run_queries: boolean;
- refingerprint: boolean;
+ auto_run_queries: boolean | null;
+ refingerprint: boolean | null;
cache_ttl: number | null;
is_sample: boolean;
is_full_sync: boolean;
diff --git a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx
index 8951f79b348..8fbf0259057 100644
--- a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx
+++ b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.jsx
@@ -9,17 +9,15 @@ import { updateIn } from "icepick";
import title from "metabase/hoc/Title";
-import Button from "metabase/core/components/Button";
import Breadcrumbs from "metabase/components/Breadcrumbs";
import Sidebar from "metabase/admin/databases/components/DatabaseEditApp/Sidebar/Sidebar";
-import DatabaseEngineWarning from "metabase/databases/containers/DatabaseEngineWarning";
import { getUserIsAdmin } from "metabase/selectors/user";
import { getWritebackEnabled } from "metabase/writeback/selectors";
-import Databases from "metabase/entities/databases";
import { getSetting } from "metabase/selectors/settings";
import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
+import DatabaseForm from "metabase/databases/containers/DatabaseForm";
import Database from "metabase-lib/metadata/Database";
import { getEditingDatabase, getInitializeError } from "../selectors";
@@ -44,8 +42,6 @@ import {
DatabaseEditRoot,
} from "./DatabaseEditApp.styled";
-const DATABASE_FORM_NAME = "database";
-
const mapStateToProps = state => {
const database = getEditingDatabase(state);
@@ -142,59 +138,16 @@ class DatabaseEditApp extends Component {
loading={!database}
error={initializeError}
>
- {() => (
- <Databases.Form
- database={database}
- form={Databases.forms.details}
- formName={DATABASE_FORM_NAME}
- onSubmit={handleSubmit}
- submitTitle={addingNewDatabase ? t`Save` : t`Save changes`}
- submitButtonComponent={Button}
- useLegacyForm
- >
- {({
- Form,
- FormField,
- FormMessage,
- FormSubmit,
- formFields,
- values,
- submitTitle,
- onChangeField,
- }) => {
- return (
- <DatabaseEditContent>
- <DatabaseEditForm>
- <Form>
- <FormField
- name="engine"
- disabled={database.is_sample}
- />
- <DatabaseEngineWarning
- engineKey={values.engine}
- onChange={engine =>
- onChangeField("engine", engine)
- }
- />
- {_.reject(formFields, { name: "engine" }).map(
- ({ name }) => (
- <FormField key={name} name={name} />
- ),
- )}
- <FormMessage />
- <div className="Form-actions text-centered">
- <FormSubmit className="block mb2">
- {submitTitle}
- </FormSubmit>
- </div>
- </Form>
- </DatabaseEditForm>
- <div>{addingNewDatabase && <DatabaseEditHelp />}</div>
- </DatabaseEditContent>
- );
- }}
- </Databases.Form>
- )}
+ <DatabaseEditContent>
+ <DatabaseEditForm>
+ <DatabaseForm
+ initialValues={database}
+ isAdvanced
+ onSubmit={handleSubmit}
+ />
+ </DatabaseEditForm>
+ <div>{addingNewDatabase && <DatabaseEditHelp />}</div>
+ </DatabaseEditContent>
</LoadingAndErrorWrapper>
</div>
</div>
diff --git a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.js b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.js
index e9b45079d92..98db6832618 100644
--- a/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.js
+++ b/frontend/src/metabase/admin/databases/containers/DatabaseEditApp.unit.spec.js
@@ -66,6 +66,8 @@ async function setup({ cachingEnabled = false } = {}) {
const settingsReducer = () => ({
values: {
+ engines: ENGINES_MOCK,
+ "enable-query-caching": cachingEnabled,
"persisted-models-enabled": false,
},
});
diff --git a/frontend/src/metabase/core/components/FormFileInput/FormFileInput.tsx b/frontend/src/metabase/core/components/FormFileInput/FormFileInput.tsx
index 1e5848aad9e..32c88d499d8 100644
--- a/frontend/src/metabase/core/components/FormFileInput/FormFileInput.tsx
+++ b/frontend/src/metabase/core/components/FormFileInput/FormFileInput.tsx
@@ -49,7 +49,6 @@ const FormFileInput = forwardRef(function FormFileInput(
style={style}
title={title}
description={description}
- orientation="horizontal"
htmlFor={id}
error={touched ? error : undefined}
>
diff --git a/frontend/src/metabase/databases/components/DatabaseCacheScheduleField/DatabaseCacheScheduleField.tsx b/frontend/src/metabase/databases/components/DatabaseCacheScheduleField/DatabaseCacheScheduleField.tsx
index 9f963d28636..1fd96f022ad 100644
--- a/frontend/src/metabase/databases/components/DatabaseCacheScheduleField/DatabaseCacheScheduleField.tsx
+++ b/frontend/src/metabase/databases/components/DatabaseCacheScheduleField/DatabaseCacheScheduleField.tsx
@@ -36,7 +36,7 @@ const DatabaseCacheScheduleField = ({
title,
description,
}: DatabaseCacheScheduleFieldProps): JSX.Element => {
- const { values, setValues } = useFormikContext<DatabaseValues>();
+ const { values, setFieldValue } = useFormikContext<DatabaseValues>();
const [{ value }, , { setValue }] = useField(name);
const handleScheduleChange = useCallback(
@@ -47,30 +47,19 @@ const DatabaseCacheScheduleField = ({
);
const handleFullSyncSelect = useCallback(() => {
- setValues(values => ({
- ...values,
- is_full_sync: true,
- is_on_demand: false,
- }));
- }, [setValues]);
+ setFieldValue("is_full_sync", true);
+ setFieldValue("is_on_demand", false);
+ }, [setFieldValue]);
const handleOnDemandSyncSelect = useCallback(() => {
- setValues(values => ({
- ...values,
- schedules: {},
- is_full_sync: false,
- is_on_demand: true,
- }));
- }, [setValues]);
+ setFieldValue("is_full_sync", false);
+ setFieldValue("is_on_demand", true);
+ }, [setFieldValue]);
const handleNoneSyncSelect = useCallback(() => {
- setValues(values => ({
- ...values,
- schedules: {},
- is_full_sync: false,
- is_on_demand: false,
- }));
- }, [setValues]);
+ setFieldValue("is_full_sync", false);
+ setFieldValue("is_on_demand", false);
+ }, [setFieldValue]);
return (
<FormField title={title} description={description}>
@@ -119,7 +108,13 @@ const ScheduleOption = ({
onSelect,
}: ScheduleOptionProps): JSX.Element => {
return (
- <ScheduleOptionRoot isSelected={isSelected} onClick={onSelect}>
+ <ScheduleOptionRoot
+ isSelected={isSelected}
+ role="option"
+ aria-label={title}
+ aria-selected={isSelected}
+ onClick={onSelect}
+ >
<ScheduleOptionIndicator isSelected={isSelected}>
<ScheduleOptionIndicatorBackground isSelected={isSelected} />
</ScheduleOptionIndicator>
diff --git a/frontend/src/metabase/databases/components/DatabaseDetailField/DatabaseDetailField.tsx b/frontend/src/metabase/databases/components/DatabaseDetailField/DatabaseDetailField.tsx
index 5cb9592678c..1b8272b0503 100644
--- a/frontend/src/metabase/databases/components/DatabaseDetailField/DatabaseDetailField.tsx
+++ b/frontend/src/metabase/databases/components/DatabaseDetailField/DatabaseDetailField.tsx
@@ -54,7 +54,8 @@ const getFieldType = (field: EngineField, override?: EngineFieldOverride) => {
};
const getFieldProps = (field: EngineField, override?: EngineFieldOverride) => {
- const placeholder = override?.placeholder ?? field.placeholder;
+ const placeholder =
+ override?.placeholder ?? field.placeholder ?? field.default;
return {
name: override?.name ?? `details.${field.name}`,
diff --git a/frontend/src/metabase/databases/components/DatabaseEngineField/DatabaseEngineField.tsx b/frontend/src/metabase/databases/components/DatabaseEngineField/DatabaseEngineField.tsx
index 5e302ca0508..ed0d0007128 100644
--- a/frontend/src/metabase/databases/components/DatabaseEngineField/DatabaseEngineField.tsx
+++ b/frontend/src/metabase/databases/components/DatabaseEngineField/DatabaseEngineField.tsx
@@ -24,8 +24,8 @@ const DatabaseEngineField = ({
const { values } = useFormikContext<DatabaseValues>();
const options = useMemo(() => {
- return getEngineOptions(engines, engineKey);
- }, [engines, engineKey]);
+ return getEngineOptions(engines, engineKey, isAdvanced);
+ }, [engines, engineKey, isAdvanced]);
return isAdvanced ? (
<DatabaseEngineSelect
diff --git a/frontend/src/metabase/databases/components/DatabaseForm/DatabaseForm.tsx b/frontend/src/metabase/databases/components/DatabaseForm/DatabaseForm.tsx
index 7df82a128be..d8e46c598b1 100644
--- a/frontend/src/metabase/databases/components/DatabaseForm/DatabaseForm.tsx
+++ b/frontend/src/metabase/databases/components/DatabaseForm/DatabaseForm.tsx
@@ -11,7 +11,11 @@ import { PLUGIN_CACHING } from "metabase/plugins";
import { Engine } from "metabase-types/api";
import { DatabaseValues } from "../../types";
import { getDefaultEngineKey } from "../../utils/engine";
-import { getValidationSchema, getVisibleFields } from "../../utils/schema";
+import {
+ getSubmitValues,
+ getValidationSchema,
+ getVisibleFields,
+} from "../../utils/schema";
import DatabaseEngineField from "../DatabaseEngineField";
import DatabaseNameField from "../DatabaseNameField";
import DatabaseDetailField from "../DatabaseDetailField";
@@ -24,7 +28,7 @@ export interface DatabaseFormProps {
isHosted?: boolean;
isAdvanced?: boolean;
isCachingEnabled?: boolean;
- onSubmit: (values: DatabaseValues) => void;
+ onSubmit?: (values: DatabaseValues) => void;
onEngineChange?: (engineKey: string | undefined) => void;
onCancel?: () => void;
}
@@ -39,20 +43,27 @@ const DatabaseForm = ({
onCancel,
onEngineChange,
}: DatabaseFormProps): JSX.Element => {
- const [engineKey, setEngineKey] = useState(() =>
- getInitialEngineKey(engines, initialData, isAdvanced),
- );
- const engine = engineKey ? engines[engineKey] : undefined;
+ const initialEngineKey = getEngineKey(engines, initialData, isAdvanced);
+ const [engineKey, setEngineKey] = useState(initialEngineKey);
+ const engine = getEngine(engines, engineKey);
const validationSchema = useMemo(() => {
return getValidationSchema(engine, engineKey, isAdvanced);
}, [engine, engineKey, isAdvanced]);
const initialValues = useMemo(() => {
- return initialData
- ? validationSchema.cast(initialData, { stripUnknown: true })
- : validationSchema.getDefault();
- }, [initialData, validationSchema]);
+ return validationSchema.cast(
+ { ...initialData, engine: engineKey },
+ { stripUnknown: true },
+ );
+ }, [initialData, engineKey, validationSchema]);
+
+ const handleSubmit = useCallback(
+ (values: DatabaseValues) => {
+ return onSubmit?.(getSubmitValues(engine, values, isAdvanced));
+ },
+ [engine, isAdvanced, onSubmit],
+ );
const handleEngineChange = useCallback(
(engineKey: string | undefined) => {
@@ -67,7 +78,7 @@ const DatabaseForm = ({
initialValues={initialValues}
validationSchema={validationSchema}
enableReinitialize
- onSubmit={onSubmit}
+ onSubmit={handleSubmit}
>
<DatabaseFormBody
engine={engine}
@@ -104,14 +115,14 @@ const DatabaseFormBody = ({
onEngineChange,
onCancel,
}: DatabaseFormBodyProps): JSX.Element => {
- const { values, dirty } = useFormikContext<DatabaseValues>();
+ const { values } = useFormikContext<DatabaseValues>();
const fields = useMemo(() => {
return engine ? getVisibleFields(engine, values, isAdvanced) : [];
}, [engine, values, isAdvanced]);
return (
- <Form disabled={isAdvanced && !dirty}>
+ <Form>
<DatabaseEngineField
engineKey={engineKey}
engines={engines}
@@ -143,17 +154,13 @@ const DatabaseFormFooter = ({
isAdvanced,
onCancel,
}: DatabaseFormFooterProps) => {
- const { values, dirty } = useFormikContext<DatabaseValues>();
+ const { values } = useFormikContext<DatabaseValues>();
const isNew = values.id == null;
if (isAdvanced) {
return (
<div>
- <FormSubmitButton
- title={isNew ? t`Save` : t`Save changes`}
- disabled={!dirty}
- primary
- />
+ <FormSubmitButton title={isNew ? t`Save` : t`Save changes`} primary />
<FormErrorMessage />
</div>
);
@@ -176,7 +183,11 @@ const DatabaseFormFooter = ({
}
};
-const getInitialEngineKey = (
+const getEngine = (engines: Record<string, Engine>, engineKey?: string) => {
+ return engineKey ? engines[engineKey] : undefined;
+};
+
+const getEngineKey = (
engines: Record<string, Engine>,
values?: DatabaseValues,
isAdvanced?: boolean,
diff --git a/frontend/src/metabase/databases/components/DatabaseScheduleToggleField/DatabaseScheduleToggleField.tsx b/frontend/src/metabase/databases/components/DatabaseScheduleToggleField/DatabaseScheduleToggleField.tsx
index 2d0e010aabe..bc9121ae6e8 100644
--- a/frontend/src/metabase/databases/components/DatabaseScheduleToggleField/DatabaseScheduleToggleField.tsx
+++ b/frontend/src/metabase/databases/components/DatabaseScheduleToggleField/DatabaseScheduleToggleField.tsx
@@ -14,20 +14,14 @@ const DatabaseScheduleToggleField = ({
title,
description,
}: DatabaseScheduleToggleFieldProps): JSX.Element => {
- const { setValues } = useFormikContext<DatabaseValues>();
+ const { setFieldValue } = useFormikContext<DatabaseValues>();
const handleChange = useCallback(
(value: boolean) => {
- if (!value) {
- setValues(values => ({
- ...values,
- schedules: {},
- is_full_sync: true,
- is_on_demand: false,
- }));
- }
+ setFieldValue("is_full_sync", !value);
+ setFieldValue("is_on_demand", false);
},
- [setValues],
+ [setFieldValue],
);
return (
diff --git a/frontend/src/metabase/databases/types.ts b/frontend/src/metabase/databases/types.ts
index 4cd8c417bd8..ce1cb4275b2 100644
--- a/frontend/src/metabase/databases/types.ts
+++ b/frontend/src/metabase/databases/types.ts
@@ -12,8 +12,8 @@ export interface DatabaseValues {
engine: string | undefined;
details: Record<string, unknown>;
schedules: DatabaseSchedules;
- auto_run_queries: boolean;
- refingerprint: boolean;
+ auto_run_queries: boolean | null;
+ refingerprint: boolean | null;
cache_ttl: number | null;
is_sample: boolean;
is_full_sync: boolean;
diff --git a/frontend/src/metabase/databases/utils/engine.ts b/frontend/src/metabase/databases/utils/engine.ts
index af754e4b4c1..c5e4db84891 100644
--- a/frontend/src/metabase/databases/utils/engine.ts
+++ b/frontend/src/metabase/databases/utils/engine.ts
@@ -4,26 +4,37 @@ import { EngineOption } from "../types";
export const getEngineOptions = (
engines: Record<string, Engine>,
- engineKey?: string,
+ selectedKey?: string,
+ isAdvanced?: boolean,
): EngineOption[] => {
- return Object.entries(engines)
- .filter(([key, engine]) => key === engineKey || !engine["superseded-by"])
- .map(([key, engine]) => ({
- name: engine["driver-name"],
- value: key,
- index: ELEVATED_ENGINES.indexOf(key),
- }))
- .sort((a, b) => {
- if (a.index >= 0 && b.index >= 0) {
- return a.index - b.index;
- } else if (a.index >= 0) {
- return -1;
- } else if (b.index >= 0) {
- return 1;
- } else {
- return a.name.localeCompare(b.name);
- }
- });
+ const options = Object.entries(engines)
+ .filter(([key, engine]) => isEngineVisible(key, engine, selectedKey))
+ .map(([key, engine]) => getEngineOption(key, engine))
+ .sort((a, b) => a.name.localeCompare(b.name));
+
+ return isAdvanced ? options : options.sort((a, b) => a.index - b.index);
+};
+
+const isEngineVisible = (
+ engineKey: string,
+ engine: Engine,
+ selectedEngineKey?: string,
+) => {
+ const isSelected = engineKey === selectedEngineKey;
+ const isSuperseded = engine["superseded-by"] != null;
+ const isSuperseding = engine["superseded-by"] === selectedEngineKey;
+
+ return isSelected || !isSuperseded || isSuperseding;
+};
+
+const getEngineOption = (engineKey: string, engine: Engine) => {
+ const index = ELEVATED_ENGINES.indexOf(engineKey);
+
+ return {
+ name: engine["driver-name"],
+ value: engineKey,
+ index: index >= 0 ? index : ELEVATED_ENGINES.length,
+ };
};
export const getEngineLogo = (engine: string): string | undefined => {
diff --git a/frontend/src/metabase/databases/utils/schema.ts b/frontend/src/metabase/databases/utils/schema.ts
index 9f41eb640b4..c0fedc2b7dd 100644
--- a/frontend/src/metabase/databases/utils/schema.ts
+++ b/frontend/src/metabase/databases/utils/schema.ts
@@ -1,28 +1,37 @@
import * as Yup from "yup";
import type { TestContext } from "yup";
import * as Errors from "metabase/core/utils/errors";
-import { Engine, EngineField } from "metabase-types/api";
+import { Engine, EngineField, ScheduleType } from "metabase-types/api";
import { ADVANCED_FIELDS, FIELD_OVERRIDES } from "../constants";
import { DatabaseValues } from "../types";
+const SCHEDULE_SCHEMA = Yup.object({
+ schedule_type: Yup.mixed().nullable(),
+ schedule_day: Yup.mixed().nullable(),
+ schedule_frame: Yup.mixed().nullable(),
+ schedule_hour: Yup.number().nullable(),
+ schedule_minute: Yup.number().nullable(),
+});
+
export const getValidationSchema = (
engine: Engine | undefined,
engineKey: string | undefined,
isAdvanced: boolean,
) => {
- const fields = getFields(engine, isAdvanced).filter(isDetailField);
+ const fields = getDefinedFields(engine, isAdvanced).filter(isDetailField);
const entries = fields.map(field => [field.name, getFieldSchema(field)]);
return Yup.object({
+ id: Yup.number(),
engine: Yup.string().default(engineKey).required(Errors.required),
name: Yup.string().default("").required(Errors.required),
details: Yup.object(Object.fromEntries(entries)),
schedules: Yup.object({
- metadata_sync: Yup.object(),
- cache_field_values: Yup.object(),
+ metadata_sync: SCHEDULE_SCHEMA.default(undefined),
+ cache_field_values: SCHEDULE_SCHEMA.default(undefined),
}),
- auto_run_queries: Yup.boolean().default(true),
- refingerprint: Yup.boolean().default(false),
+ auto_run_queries: Yup.boolean().nullable().default(true),
+ refingerprint: Yup.boolean().nullable().default(false),
cache_ttl: Yup.number().nullable().default(null).positive(Errors.positive),
is_sample: Yup.boolean().default(false),
is_full_sync: Yup.boolean().default(false),
@@ -31,15 +40,18 @@ export const getValidationSchema = (
};
export const getVisibleFields = (
- engine: Engine,
+ engine: Engine | undefined,
values: DatabaseValues,
isAdvanced: boolean,
) => {
- const fields = getFields(engine, isAdvanced);
+ const fields = getDefinedFields(engine, isAdvanced);
return fields.filter(field => isFieldVisible(field, values.details));
};
-const getFields = (engine: Engine | undefined, isAdvanced: boolean) => {
+export const getDefinedFields = (
+ engine: Engine | undefined,
+ isAdvanced: boolean,
+) => {
const fields = engine?.["details-fields"] ?? [];
return isAdvanced
@@ -47,24 +59,46 @@ const getFields = (engine: Engine | undefined, isAdvanced: boolean) => {
: fields.filter(field => !ADVANCED_FIELDS.includes(field.name));
};
+export const getSubmitValues = (
+ engine: Engine | undefined,
+ values: DatabaseValues,
+ isAdvanced: boolean,
+) => {
+ const fields = getVisibleFields(engine, values, isAdvanced);
+ const entries = fields
+ .filter(field => isDetailField(field))
+ .filter(field => isFieldVisible(field, values.details))
+ .map(field => [field.name, values.details[field.name]]);
+
+ return {
+ ...values,
+ details: Object.fromEntries(entries),
+ };
+};
+
const getFieldSchema = (field: EngineField) => {
switch (field.type) {
case "integer":
return Yup.number()
.nullable()
- .default(field.default != null ? Number(field.default) : null)
+ .default(null)
.test((value, context) => isFieldValid(field, value, context));
case "boolean":
case "section":
return Yup.boolean()
- .defined()
+ .nullable()
.default(field.default != null ? Boolean(field.default) : false)
.test((value, context) => isFieldValid(field, value, context));
- default:
+ case "select":
return Yup.string()
.nullable()
.default(field.default != null ? String(field.default) : null)
.test((value, context) => isFieldValid(field, value, context));
+ default:
+ return Yup.string()
+ .nullable()
+ .default(null)
+ .test((value, context) => isFieldValid(field, value, context));
}
};
diff --git a/frontend/test/metabase/scenarios/admin/databases/add-external.cy.spec.js b/frontend/test/metabase/scenarios/admin/databases/add-external.cy.spec.js
index 7c8ba4fb386..4258241b864 100644
--- a/frontend/test/metabase/scenarios/admin/databases/add-external.cy.spec.js
+++ b/frontend/test/metabase/scenarios/admin/databases/add-external.cy.spec.js
@@ -11,7 +11,7 @@ describe(
cy.intercept("POST", "/api/database").as("createDatabase");
cy.visit("/admin/databases/create");
- cy.contains("Database type").closest(".Form-field").find("a").click();
+ cy.findByLabelText("Database type").click();
});
it("should add Postgres database and redirect to listing (metabase#12972, metabase#14334, metabase#17450)", () => {
@@ -31,14 +31,18 @@ describe(
.click()
.should("have.attr", "aria-checked", "true");
- isSyncOptionSelected("Never, I'll do this manually if I need to");
+ cy.findByLabelText("Never, I'll do this manually if I need to").should(
+ "have.attr",
+ "aria-selected",
+ "true",
+ );
// make sure fields needed to connect to the database are properly trimmed (metabase#12972)
typeAndBlurUsingLabel("Display name", "QA Postgres12");
- typeAndBlurUsingLabel("Host", "localhost \n ");
+ typeAndBlurUsingLabel("Host", "localhost");
typeAndBlurUsingLabel("Port", "5432");
- typeAndBlurUsingLabel("Database name", " sample");
- typeAndBlurUsingLabel("Username", " metabase ");
+ typeAndBlurUsingLabel("Database name", "sample");
+ typeAndBlurUsingLabel("Username", "metabase");
typeAndBlurUsingLabel("Password", "metasample123");
cy.button("Save").should("not.be.disabled").click();
@@ -69,7 +73,11 @@ describe(
"true",
);
- isSyncOptionSelected("Never, I'll do this manually if I need to");
+ cy.findByLabelText("Never, I'll do this manually if I need to").should(
+ "have.attr",
+ "aria-selected",
+ "true",
+ );
});
it("should add Mongo database and redirect to listing", () => {
@@ -137,8 +145,3 @@ describe(
});
},
);
-
-function isSyncOptionSelected(option) {
- // This is a really bad way to assert that the text element is selected/active. Can it be fixed in the FE code?
- cy.findByText(option).parent().should("have.class", "text-brand");
-}
diff --git a/frontend/test/metabase/scenarios/admin/databases/add-presto.cy.spec.js b/frontend/test/metabase/scenarios/admin/databases/add-presto.cy.spec.js
index ca7a84ce777..2ab2a54470e 100644
--- a/frontend/test/metabase/scenarios/admin/databases/add-presto.cy.spec.js
+++ b/frontend/test/metabase/scenarios/admin/databases/add-presto.cy.spec.js
@@ -6,7 +6,7 @@ describe("admin > database > add > Presto", () => {
cy.signInAsAdmin();
cy.visit("/admin/databases/create");
- cy.contains("Database type").closest(".Form-field").find("a").click();
+ cy.findByLabelText("Database type").click();
});
it("should render correctly and allow switching between the new and the old drivers (metabase#18351)", () => {
@@ -44,13 +44,13 @@ describe("admin > database > add > Presto", () => {
cy.findByLabelText("Choose when syncs and scans happen").should(
"have.attr",
"aria-checked",
- "",
+ "false",
);
cy.findByLabelText("Periodically refingerprint tables").should(
"have.attr",
"aria-checked",
- "",
+ "false",
);
// This should be disabled but we'll not add that assertion until we mark all the required fields in the form
@@ -69,9 +69,6 @@ describe("admin > database > add > Presto", () => {
cy.findAllByTestId("select-button").contains("Presto (Deprecated Driver)");
- // It should have persisted the previously set database name
- cy.findByDisplayValue("Foo");
-
cy.findByLabelText("Host");
cy.findByLabelText("Port");
cy.findByLabelText("Catalog");
@@ -79,6 +76,7 @@ describe("admin > database > add > Presto", () => {
cy.findByLabelText("Schema (optional)").should("not.exist");
cy.findByLabelText("Username");
cy.findByLabelText("Password");
+ cy.findByText("Show advanced options").click();
// Reproduces metabase#18351
cy.findByLabelText("Additional JDBC options").should("not.exist");
@@ -95,13 +93,13 @@ describe("admin > database > add > Presto", () => {
cy.findByLabelText("Choose when syncs and scans happen").should(
"have.attr",
"aria-checked",
- "",
+ "false",
);
cy.findByLabelText("Periodically refingerprint tables").should(
"have.attr",
"aria-checked",
- "",
+ "false",
);
cy.contains("This driver will be removed in a future release. ");
diff --git a/frontend/test/metabase/scenarios/admin/databases/add.cy.spec.js b/frontend/test/metabase/scenarios/admin/databases/add.cy.spec.js
index 0bfcc0db4de..be51e9a3510 100644
--- a/frontend/test/metabase/scenarios/admin/databases/add.cy.spec.js
+++ b/frontend/test/metabase/scenarios/admin/databases/add.cy.spec.js
@@ -55,10 +55,7 @@ describe("scenarios > admin > databases > add", () => {
cy.onlyOn(isEE);
cy.visit("/admin/databases/create");
- cy.contains("Database type")
- .closest(".Form-field")
- .findByTestId("select-button")
- .click();
+ cy.findByLabelText("Database type").click();
popover().within(() => {
cy.findByText("Oracle");
cy.findByText("Vertica");
@@ -108,31 +105,6 @@ describe("scenarios > admin > databases > add", () => {
});
});
- it("should show the old BigQuery form for previously connected databases", () => {
- cy.intercept("GET", "/api/database/123", req => {
- req.reply({
- statusCode: 200,
- body: {
- id: 123,
- engine: "bigquery",
- details: {
- "auth-code": "auth-code",
- "client-id": "client-id",
- "client-secret": "client-secret",
- "dataset-id": "dataset-id",
- "project-id": "project",
- "use-jvm-timezone": false,
- },
- },
- delay: 100,
- });
- });
- cy.visit("/admin/databases/123");
-
- cy.contains("Connect to a Service Account instead");
- cy.contains("generate a Client ID and Client Secret for your project");
- });
-
it("should display driver deprecation messages", () => {
cy.visit("/admin/databases/create");
@@ -264,14 +236,11 @@ describe("scenarios > admin > databases > add", () => {
});
});
function toggleFieldWithDisplayName(displayName) {
- cy.contains(displayName).closest(".Form-field").find("input").click();
+ cy.findByLabelText(displayName).click();
}
function selectFieldOption(fieldName, option) {
- cy.contains(fieldName)
- .parents(".Form-field")
- .findByTestId("select-button")
- .click();
+ cy.findByLabelText(fieldName).click();
popover().contains(option).click({ force: true });
}
diff --git a/frontend/test/metabase/scenarios/admin/databases/edit.cy.spec.js b/frontend/test/metabase/scenarios/admin/databases/edit.cy.spec.js
index 712fe6e9d64..6630cb2e10d 100644
--- a/frontend/test/metabase/scenarios/admin/databases/edit.cy.spec.js
+++ b/frontend/test/metabase/scenarios/admin/databases/edit.cy.spec.js
@@ -124,29 +124,30 @@ describe("scenarios > admin > databases > edit", () => {
cy.visit(`/admin/databases/${SAMPLE_DB_ID}`);
cy.findByText("Show advanced options").click();
- cy.findByText("Database syncing")
- .closest(".Form-field")
- .findByText("Hourly");
-
- cy.findByText("Regularly, on a schedule")
- .closest("div")
- .should("have.class", "text-brand");
+ cy.findByText("Regularly, on a schedule").should("exist");
+ cy.findByText("Hourly").should("exist");
+ cy.findByLabelText("Regularly, on a schedule").should(
+ "have.attr",
+ "aria-selected",
+ "true",
+ );
});
it("lets you change the metadata_sync period", () => {
cy.visit(`/admin/databases/${SAMPLE_DB_ID}`);
cy.findByText("Show advanced options").click();
- cy.findByText("Database syncing").closest(".Form-field").as("sync");
- cy.get("@sync").findByText("Hourly").click();
+ cy.findByText("Hourly").click();
popover().within(() => {
cy.findByText("Daily").click({ force: true });
});
- cy.findByText("Regularly, on a schedule")
- .closest("div")
- .should("have.class", "text-brand");
+ cy.findByLabelText("Regularly, on a schedule").should(
+ "have.attr",
+ "aria-selected",
+ "true",
+ );
cy.findByText("Save changes").click();
cy.wait("@databaseUpdate").then(({ response }) =>
--
GitLab