diff --git a/.circleci/config.yml b/.circleci/config.yml
index 59b9c85b047709719c4381f832caecb4d185de08..6f4181a5efa6743ef6fae03aefd8d7f9244c95c7 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -170,6 +170,11 @@ executors:
ACCEPT_EULA: Y
SA_PASSWORD: 'P@ssw0rd'
+ fe-mongo:
+ working_directory: /home/circleci/metabase/metabase/
+ docker:
+ - image: circleci/clojure:lein-2.8.1-node-browsers
+ - image: circleci/mongo:4.0
########################################################################################################################
@@ -279,6 +284,13 @@ jobs:
done;
echo `md5sum yarn.lock` >> frontend-checksums.txt
echo `md5sum webpack.config.js` >> frontend-checksums.txt
+ # As well as driver modules (database drivers)
+ - run:
+ name: Generate checksums of all driver module source files to use as Uberjar cache key
+ command: >
+ for file in `find ./modules -type f | sort`;
+ do echo `md5sum $file` >> modules-checksums.txt;
+ done;
- run:
name: Save last git commit message
command: git log -1 --oneline > commit.txt
@@ -530,18 +542,18 @@ jobs:
- restore-be-deps-cache
- restore_cache:
keys:
- - uberjar-{{ checksum "./backend-checksums.txt" }}-{{ checksum "./frontend-checksums.txt" }}
+ - uberjar-{{ checksum "./backend-checksums.txt" }}-{{ checksum "./frontend-checksums.txt" }}-{{ checksum "./modules-checksums.txt" }}
- run:
name: Build uberjar if needed
command: >
if [ ! -f './target/uberjar/metabase.jar' ];
- then ./bin/build version frontend uberjar;
+ then ./bin/build version frontend drivers uberjar;
fi
no_output_timeout: 5m
- store_artifacts:
path: /home/circleci/metabase/metabase/target/uberjar/metabase.jar
- save_cache:
- key: uberjar-{{ checksum "./backend-checksums.txt" }}-{{ checksum "./frontend-checksums.txt" }}
+ key: uberjar-{{ checksum "./backend-checksums.txt" }}-{{ checksum "./frontend-checksums.txt" }}-{{ checksum "./modules-checksums.txt" }}
paths:
- /home/circleci/metabase/metabase/target/uberjar/metabase.jar
@@ -560,11 +572,24 @@ jobs:
command: ./bin/build version
fe-tests-cypress:
- executor: clojure-and-node
+ parameters:
+ e:
+ type: executor
+ default: clojure-and-node
+ only-single-database:
+ type: boolean
+ default: false
+ test-files-location:
+ type: string
+ default: ""
+ driver:
+ type: string
+ default: ""
+ executor: << parameters.e >>
steps:
- run-yarn-command:
command-name: Run Cypress tests
- command: run test-cypress-no-build
+ command: run test-cypress-no-build <<# parameters.only-single-database >> --testFiles << parameters.test-files-location >> <</ parameters.only-single-database >>
before-steps:
- restore_cache:
keys:
@@ -852,6 +877,16 @@ workflows:
- build-uberjar
- fe-deps
+ - fe-tests-cypress:
+ name: fe-tests-cypress-mongo
+ requires:
+ - build-uberjar
+ - fe-deps
+ e: fe-mongo
+ driver: mongo
+ only-single-database: true
+ test-files-location: frontend/test/metabase-db/mongo
+
- deploy-master:
requires:
- yaml-linter
diff --git a/backend/mbql/src/metabase/mbql/normalize.clj b/backend/mbql/src/metabase/mbql/normalize.clj
index 841b32da7a432aaeab86a89dc32cf1e5922c534c..19630c088e9438722e646ee8b5aa2a967789f5ac 100644
--- a/backend/mbql/src/metabase/mbql/normalize.clj
+++ b/backend/mbql/src/metabase/mbql/normalize.clj
@@ -231,6 +231,13 @@
(update :special_type keyword)
(update :fingerprint walk/keywordize-keys)))
+(defn- normalize-native-query
+ "For native queries, normalize the top-level keys, and template tags, but nothing else."
+ [native-query]
+ (let [native-query (m/map-keys mbql.u/normalize-token native-query)]
+ (cond-> native-query
+ (seq (:template-tags native-query)) (update :template-tags normalize-template-tags))))
+
;; TODO - why not make this a multimethod of some sort?
(def ^:private path->special-token-normalization-fn
"Map of special functions that should be used to perform token normalization for a given path. For example, the
@@ -238,9 +245,7 @@
defined below."
{:type mbql.u/normalize-token
;; don't normalize native queries
- :native {:query identity
- :template-tags normalize-template-tags
- :params identity}
+ :native normalize-native-query
:query {:aggregation normalize-ag-clause-tokens
:expressions normalize-expressions-tokens
:order-by normalize-order-by-tokens
diff --git a/backend/mbql/test/metabase/mbql/normalize_test.clj b/backend/mbql/test/metabase/mbql/normalize_test.clj
index ac2f8697701270d67c34b4e8858e14f1f90f5080..1da4454005cb866d828317dc56e571f6ac5b12f0 100644
--- a/backend/mbql/test/metabase/mbql/normalize_test.clj
+++ b/backend/mbql/test/metabase/mbql/normalize_test.clj
@@ -428,6 +428,14 @@
:params ["Red Medicine"]}}))
":native :params shouldn't get normalized."))
+(deftest normalize-projections-test
+ (testing "Native :projections shouldn't get normalized."
+ (is (= {:type :native
+ :native {:projections ["_id" "name" "category_id" "latitude" "longitude" "price"]}}
+ (#'normalize/normalize-tokens
+ {:type :native
+ :native {:projections ["_id" "name" "category_id" "latitude" "longitude" "price"]}})))))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | CANONICALIZE |
diff --git a/bin/version b/bin/version
index ec26e64ca4640c1208962d96a1bc0d11525c5f12..5a79a4f74bed66e715dca014a10a5520f4f4b047 100755
--- a/bin/version
+++ b/bin/version
@@ -1,6 +1,6 @@
#!/usr/bin/env bash
-VERSION="v0.35.0"
+VERSION="v0.35.2"
# dynamically pull more interesting stuff from latest git commit
HASH=$(git show-ref --head --hash=7 head) # first 7 letters of hash should be enough; that's what GitHub uses
diff --git a/docs/users-guide/expressions.md b/docs/users-guide/expressions.md
index 60e05948b582af8cec8845302895fd42968b8ea0..5db02368f94f4c09e39d203f9ae7c986b94f910c 100644
--- a/docs/users-guide/expressions.md
+++ b/docs/users-guide/expressions.md
@@ -76,7 +76,7 @@ This would return rows where `Created At` is between January 1, 2020 and March 3
| Percentile | `percentile(column, percentile-value)` | Returns the value of the column at the percentile value. | `percentile([Score], 0.9)` |
| Power | `power(number, exponent)` | Raises a number to the power of the exponent value. | `power([Length], 2)` |
| Regex extract | `regexextract(text, regular_expression)` | Extracts matching substrings according to a regular expression. | `regexextract( [Address], "[0-9]+" )` |
-| Replace | `replace(text, position, length, new_text)` | Replaces a part of the input text with new text. | `replace( [Order ID], 8, 3, [Updated Part of ID] )` |
+| Replace | `replace(text, find, replace)` | Replaces a part of the input text with new text. | `replace( [Title], "Enormous", "Gigantic" )` |
| Round | `round(number)` | Rounds a decimal number either up or down to the nearest integer value. | `round([Temperature])` |
| Right trim | `rtrim(text)` | Removes trailing whitespace from a string of text. | `rtrim( [Comment] )` |
| Share | `share(condition)` | Returns the percent of rows in the data that match the condition, as a decimal. | `share( [Source] = "Goolge" )` |
@@ -94,7 +94,7 @@ This would return rows where `Created At` is between January 1, 2020 and March 3
Certain database types don't support some of the above functions:
-MySQL
+MySQL and SQL Server
- median
- percentile
diff --git a/frontend/src/metabase/admin/permissions/routes.jsx b/frontend/src/metabase/admin/permissions/routes.jsx
index 1f6a667d0cba6062d7c8784e7aeb40c6d5695f9f..b9ba8ae07726075aed877b27fc8c37240e656b6d 100644
--- a/frontend/src/metabase/admin/permissions/routes.jsx
+++ b/frontend/src/metabase/admin/permissions/routes.jsx
@@ -31,7 +31,7 @@ const getRoutes = store => (
</Route>
{/* TABLES NO SCHEMA */}
- {/* NOTE: this route is to support null schemas, inject the empty string as the schemaName */}
+ {/* NOTE: this route is to support null schemas */}
<Route
path=":databaseId/tables"
component={(
@@ -39,7 +39,7 @@ const getRoutes = store => (
) => (
<TablesPermissionsApp
{...props}
- params={{ ...props.params, schemaName: "" }}
+ params={{ ...props.params, schemaName: null }}
/>
)}
>
diff --git a/frontend/src/metabase/admin/permissions/selectors.js b/frontend/src/metabase/admin/permissions/selectors.js
index 965b36cfa0a7e7b5fbcdcf5487091a3900993c90..9ff1b4be987ea8288259a47b83b7666ac3a4c1d7 100644
--- a/frontend/src/metabase/admin/permissions/selectors.js
+++ b/frontend/src/metabase/admin/permissions/selectors.js
@@ -662,7 +662,7 @@ export const getDatabasesPermissionsGrid = createSelector(
},
name: database.name,
link:
- schemas.length === 0 || (schemas.length === 1 && schemas[0] === "")
+ schemas.length === 0 || (schemas.length === 1 && schemas[0] == null)
? {
name: t`View tables`,
url: `/admin/permissions/databases/${database.id}/tables`,
diff --git a/frontend/src/metabase/admin/settings/components/SettingsBatchForm.jsx b/frontend/src/metabase/admin/settings/components/SettingsBatchForm.jsx
index b716604837c3e423a4c04791f5eac6cbe202e631..4a2c220dd3bb0a0c7b05aeeb798736a0c5dadb27 100644
--- a/frontend/src/metabase/admin/settings/components/SettingsBatchForm.jsx
+++ b/frontend/src/metabase/admin/settings/components/SettingsBatchForm.jsx
@@ -33,6 +33,8 @@ const SAVE_SETTINGS_BUTTONS_STATES = {
@connect(
null,
{ updateSettings },
+ null,
+ { withRef: true }, // HACK: needed so consuming components can call methods on the component :-/
)
export default class SettingsBatchForm extends Component {
constructor(props, context) {
diff --git a/frontend/src/metabase/admin/settings/components/SettingsEmailForm.jsx b/frontend/src/metabase/admin/settings/components/SettingsEmailForm.jsx
index 9c77536225dedaad03cfbca60c2d463a59912779..0afdb3a0e7f6dfc4f378520d2e20e6893a1f0592 100644
--- a/frontend/src/metabase/admin/settings/components/SettingsEmailForm.jsx
+++ b/frontend/src/metabase/admin/settings/components/SettingsEmailForm.jsx
@@ -69,7 +69,7 @@ export default class SettingsEmailForm extends Component {
const { sendingEmail } = this.state;
return (
<SettingsBatchForm
- ref={form => (this._form = form)}
+ ref={form => (this._form = form && form.getWrappedInstance())}
{...this.props}
updateSettings={this.props.updateEmailSettings}
disable={sendingEmail !== "default"}
diff --git a/frontend/src/metabase/admin/settings/containers/SettingsEditorApp.jsx b/frontend/src/metabase/admin/settings/containers/SettingsEditorApp.jsx
index 320322288c7f937e92303484f19e51c4b3c7bbd0..413ab6cba3df738659334bdee9460f15f4c0a80a 100644
--- a/frontend/src/metabase/admin/settings/containers/SettingsEditorApp.jsx
+++ b/frontend/src/metabase/admin/settings/containers/SettingsEditorApp.jsx
@@ -109,6 +109,7 @@ export default class SettingsEditorApp extends Component {
const { settings, updateSetting } = this.props;
const setting = _.findWhere(settings, { key });
if (!setting) {
+ console.error(`Attempted to change unknown setting ${key}`);
throw new Error(t`Unknown setting ${key}`);
}
return updateSetting({ ...setting, value });
diff --git a/frontend/src/metabase/containers/Form.jsx b/frontend/src/metabase/containers/Form.jsx
index 5e6adcc0415a0f3a288255df21b907634dbfdc5d..2784d1299eca09970839baa2e8efb63fb37b4f5f 100644
--- a/frontend/src/metabase/containers/Form.jsx
+++ b/frontend/src/metabase/containers/Form.jsx
@@ -8,6 +8,7 @@ import { createSelector } from "reselect";
import { reduxForm, getValues, initialize, change } from "redux-form";
import { getIn, assocIn } from "icepick";
import _ from "underscore";
+import { t } from "ttag";
import CustomForm from "metabase/components/form/CustomForm";
import StandardForm from "metabase/components/form/StandardForm";
@@ -82,6 +83,12 @@ type State = {
inlineFields: { [name: FormFieldName]: FormFieldDefinition },
};
+type SubmitState = {
+ submitting: boolean,
+ failed: boolean,
+ result: any,
+};
+
let FORM_ID = 0;
// use makeMapStateToProps so each component gets it's own unique formId
const makeMapStateToProps = () => {
@@ -95,19 +102,37 @@ const makeMapStateToProps = () => {
};
};
-const ReduxFormComponent = reduxForm()(props => {
- const FormComponent =
- props.formComponent || (props.children ? CustomForm : StandardForm);
- return <FormComponent {...props} />;
-});
+const ReduxFormComponent = reduxForm()(
+ ({ handleSubmit, submitState, ...props }) => {
+ const FormComponent =
+ props.formComponent || (props.children ? CustomForm : StandardForm);
+ return (
+ <FormComponent
+ {...props}
+ handleSubmit={async (...args) => {
+ await handleSubmit(...args);
+ // normally handleSubmit swallows the result/error, but we want to make it available to things like ActionButton
+ if (submitState.failed) {
+ throw submitState.result;
+ } else {
+ return submitState.result;
+ }
+ }}
+ />
+ );
+ },
+);
@connect(makeMapStateToProps)
export default class Form extends React.Component {
props: Props;
state: State;
- _submitting: boolean = false;
- _submitFailed: boolean = false;
+ _state: SubmitState = {
+ submitting: false,
+ failed: false,
+ result: undefined,
+ };
_getFormDefinition: () => FormDefinition;
_getFormObject: () => FormObject;
@@ -232,8 +257,8 @@ export default class Form extends React.Component {
_validate = (values: FormValues, props: any) => {
// HACK: clears failed state for global error
- if (!this._submitting && this._submitFailed) {
- this._submitFailed = false;
+ if (!this._state.submitting && this._state.failed) {
+ this._state.failed = false;
props.stopSubmit();
}
const formObject = this._getFormObject();
@@ -243,27 +268,35 @@ export default class Form extends React.Component {
_onSubmit = async (values: FormValues) => {
const formObject = this._getFormObject();
// HACK: clears failed state for global error
- this._submitting = true;
+ this._state.submitting = true;
try {
const normalized = formObject.normalize(values);
- return await this.props.onSubmit(normalized);
+ return (this._state.result = await this.props.onSubmit(normalized));
} catch (error) {
console.error("Form submission error", error);
- this._submitFailed = true;
+ this._state.failed = true;
+ this._state.result = error;
// redux-form expects { "FIELD NAME": "FIELD ERROR STRING" } or {"_error": "GLOBAL ERROR STRING" }
if (error && error.data && error.data.errors) {
try {
// HACK: blur the current element to ensure we show the error
document.activeElement.blur();
} catch (e) {}
- throw error.data.errors;
+ // if there are errors for fields we don't know about then inject a generic top-level _error key
+ const fieldNames = new Set(this._getFieldNames());
+ const errorNames = Object.keys(error.data.errors);
+ const hasUnknownFields = errorNames.some(name => !fieldNames.has(name));
+ throw {
+ _error: hasUnknownFields ? t`An error occurred` : null,
+ ...error.data.errors,
+ };
} else if (error) {
throw {
_error: error.data.message || error.data,
};
}
} finally {
- setTimeout(() => (this._submitting = false));
+ setTimeout(() => (this._state.submitting = false));
}
};
@@ -289,6 +322,8 @@ export default class Form extends React.Component {
validate={this._validate}
onSubmit={this._onSubmit}
onChangeField={this._handleChangeField}
+ // HACK: _state is a mutable object so we can pass by reference into the ReduxFormComponent
+ submitState={this._state}
/>
);
}
diff --git a/frontend/src/metabase/lib/expressions/config.js b/frontend/src/metabase/lib/expressions/config.js
index 5eb511e5032d7e308d52e696f4fc2e821bf69aff..5b8a8d30ceb79ecba715a199d369c9f9b95b6df5 100644
--- a/frontend/src/metabase/lib/expressions/config.js
+++ b/frontend/src/metabase/lib/expressions/config.js
@@ -177,7 +177,7 @@ export const MBQL_CLAUSES = {
multiple: true,
},
replace: {
- displayName: `substitute`,
+ displayName: `replace`,
type: "string",
args: ["string", "string", "string"],
},
diff --git a/frontend/src/metabase/lib/expressions/helper_text_strings.js b/frontend/src/metabase/lib/expressions/helper_text_strings.js
index 34cd35972bd72a693ca52a527a294113e2c5d114..fb00110302856e01705d5776f13e97d80ff8024c 100644
--- a/frontend/src/metabase/lib/expressions/helper_text_strings.js
+++ b/frontend/src/metabase/lib/expressions/helper_text_strings.js
@@ -235,35 +235,21 @@ const helperTextStrings = [
},
{
name: "replace",
- structure:
- "replace(" +
- t`text` +
- ", " +
- t`position` +
- ", " +
- t`length` +
- ", " +
- t`new_text` +
- ")",
+ structure: "replace(" + t`text` + ", " + t`find` + ", " + t`replace` + ")",
description: t`Replaces a part of the input text with new text.`,
- example:
- "replace([" + t`Order ID` + "] , 8, 3, [" + t`Updated Part of ID` + "] )",
+ example: "replace([" + t`Title` + '] , "Enormous", "Gigantic")',
args: [
{
name: t`text`,
description: t`The text that will be modified.`,
},
{
- name: t`position`,
- description: t`The position where the replacing will start.`,
- },
- {
- name: t`length`,
- description: t`The number of characters to replace.`,
+ name: t`find`,
+ description: t`The text to find.`,
},
{
- name: t`new_text`,
- description: t`The text to use in the replacement.`,
+ name: t`replace`,
+ description: t`The text to use as the replacement.`,
},
],
},
diff --git a/frontend/src/metabase/lib/query/field_ref.js b/frontend/src/metabase/lib/query/field_ref.js
index 44332da6e42f75b05d649f2f5163a743cab98427..b3523d3a210e36f90437bd1fe41d0fb6a385528c 100644
--- a/frontend/src/metabase/lib/query/field_ref.js
+++ b/frontend/src/metabase/lib/query/field_ref.js
@@ -62,7 +62,7 @@ export function isValidField(field) {
(isAggregateField(field) && typeof field[1] === "number") ||
(isJoinedField(field) &&
typeof field[1] === "string" &&
- isValidField(field[0])) ||
+ isValidField(field[2])) ||
isFieldLiteral(field)
);
}
diff --git a/frontend/src/metabase/lib/query/filter.js b/frontend/src/metabase/lib/query/filter.js
index d85ee86c7ad0464a9cc585865ab212cd503fd160..552c558797055e4c51068063f2157ddcc953898e 100644
--- a/frontend/src/metabase/lib/query/filter.js
+++ b/frontend/src/metabase/lib/query/filter.js
@@ -70,7 +70,7 @@ export function canAddFilter(filter: ?FilterClause): boolean {
export function isStandard(filter: FilterClause): boolean {
return (
Array.isArray(filter) &&
- STANDARD_FILTERS.has(filter[0]) &&
+ (STANDARD_FILTERS.has(filter[0]) || filter[0] === null) &&
(filter[1] === undefined || isValidField(filter[1]))
);
}
diff --git a/frontend/src/metabase/parameters/components/Parameters.jsx b/frontend/src/metabase/parameters/components/Parameters.jsx
index 9048bc4335f5ccbaca595faa7ed5dad41df6ea93..4c908e3577eb45d6475c5d8362551029c8d19407 100644
--- a/frontend/src/metabase/parameters/components/Parameters.jsx
+++ b/frontend/src/metabase/parameters/components/Parameters.jsx
@@ -71,10 +71,14 @@ export default class Parameters extends Component {
const queryParam = query && query[parameter.slug];
if (queryParam != null || parameter.default != null) {
let value = queryParam != null ? queryParam : parameter.default;
- if (parameter.hasOnlyFieldTargets && value && !Array.isArray(value)) {
- // FieldValuesWidget always produces an array. If this param has
- // only field targets we'll use that widget, so we should start with
- // an array to match.
+
+ // ParameterValueWidget uses FieldValuesWidget if there's no available
+ // date widget and all targets are fields. This matches that logic.
+ const willUseFieldValuesWidget =
+ parameter.hasOnlyFieldTargets && !/^date\//.test(parameter.type);
+ if (willUseFieldValuesWidget && value && !Array.isArray(value)) {
+ // FieldValuesWidget always produces an array. If we'll use that
+ // widget, we should start with an array to match.
value = [value];
}
const fieldIds = parameter.field_ids || [];
diff --git a/frontend/src/metabase/visualizations/components/CardRenderer.jsx b/frontend/src/metabase/visualizations/components/CardRenderer.jsx
index 04313664f865c2416431cd96068f8025024ba38c..d6bcec81c39d2afa6967f746e3b119bbd601b331 100644
--- a/frontend/src/metabase/visualizations/components/CardRenderer.jsx
+++ b/frontend/src/metabase/visualizations/components/CardRenderer.jsx
@@ -108,7 +108,6 @@ export default class CardRenderer extends Component {
});
} catch (err) {
console.error(err);
- this.props.onRenderError(err.message || err);
}
}
diff --git a/frontend/src/metabase/visualizations/components/ChoroplethMap.jsx b/frontend/src/metabase/visualizations/components/ChoroplethMap.jsx
index af0e7cb4dff6305bd028864d7b73347c70d99fcd..13ec640388e79ebb75b8f8b7cd1fb27df76f9095 100644
--- a/frontend/src/metabase/visualizations/components/ChoroplethMap.jsx
+++ b/frontend/src/metabase/visualizations/components/ChoroplethMap.jsx
@@ -330,6 +330,7 @@ export default class ChoroplethMap extends Component {
onClickFeature={onClickFeature}
projection={projection}
projectionFrame={projectionFrame}
+ onRenderError={this.props.onRenderError}
/>
) : (
<LeafletChoropleth
@@ -339,6 +340,7 @@ export default class ChoroplethMap extends Component {
onHoverFeature={onHoverFeature}
onClickFeature={onClickFeature}
minimalBounds={minimalBounds}
+ onRenderError={this.props.onRenderError}
/>
)}
</ChartWithLegend>
diff --git a/frontend/src/metabase/visualizations/components/LeafletChoropleth.jsx b/frontend/src/metabase/visualizations/components/LeafletChoropleth.jsx
index d33a47f158089b25e2df9ef66254ad069211ddb8..667a5fc71307936d014c6cbe691b05755d0fce9f 100644
--- a/frontend/src/metabase/visualizations/components/LeafletChoropleth.jsx
+++ b/frontend/src/metabase/visualizations/components/LeafletChoropleth.jsx
@@ -17,6 +17,7 @@ const LeafletChoropleth = ({
getColor = () => color("brand"),
onHoverFeature = () => {},
onClickFeature = () => {},
+ onRenderError,
}) => (
<CardRenderer
series={series}
@@ -104,6 +105,7 @@ const LeafletChoropleth = ({
map.remove();
};
}}
+ onRenderError={onRenderError}
/>
);
diff --git a/frontend/test/__runner__/run_cypress_tests.js b/frontend/test/__runner__/run_cypress_tests.js
index d9415c0b9be3a3cd8dd62a7079d814486ee1cd21..d0c5d35ffef8a33e7a121235216a10feed228b76 100644
--- a/frontend/test/__runner__/run_cypress_tests.js
+++ b/frontend/test/__runner__/run_cypress_tests.js
@@ -7,8 +7,13 @@ const BackendResource = require("./backend.js").BackendResource;
const server = BackendResource.get({ dbKey: "" });
+// We currently accept two (optional) command line arguments
+// --open - Opens the Cypress test browser
+// --testFiles <path> - Specifies a different path for the integration folder
const userArgs = process.argv.slice(2);
-const isOpenMode = userArgs[0] === "--open";
+const isOpenMode = userArgs.includes("--open");
+const testFiles = userArgs.includes("--testFiles");
+const testFilesLocation = userArgs[userArgs.indexOf("--testFiles") + 1];
function readFile(fileName) {
return new Promise(function(resolve, reject) {
@@ -30,6 +35,10 @@ const init = async () => {
);
}
+ if (testFiles) {
+ console.log(chalk.bold(`Running tests in '${testFilesLocation}'`));
+ }
+
try {
const version = await readFile(
__dirname + "/../../../resources/version.properties",
@@ -57,6 +66,11 @@ const init = async () => {
await generateSnapshots();
console.log(chalk.bold("Starting Cypress"));
+ let commandLineConfig = `baseUrl=${server.host}`;
+ if (testFiles) {
+ commandLineConfig = `${commandLineConfig},integrationFolder=${testFilesLocation}`;
+ }
+
const cypressProcess = spawn(
"yarn",
[
@@ -65,7 +79,7 @@ const init = async () => {
"--config-file",
process.env["CONFIG_FILE"],
"--config",
- `baseUrl=${server.host}`,
+ commandLineConfig,
...(process.env["CI"]
? [
"--reporter",
diff --git a/frontend/test/__support__/cypress.js b/frontend/test/__support__/cypress.js
index 94c8ae103e49d9c790fa8abdca62302070d8a533..075eecfc9986c4f4bac294e7b2ef1453f4f7bc38 100644
--- a/frontend/test/__support__/cypress.js
+++ b/frontend/test/__support__/cypress.js
@@ -68,6 +68,8 @@ export function main() {
return cy.get("nav").next();
}
+// Metabase utility functions for commonly-used patterns
+
export function openOrdersTable() {
cy.visit("/question/new?database=1&table=2");
}
@@ -76,4 +78,13 @@ export function openProductsTable() {
cy.visit("/question/new?database=1&table=1");
}
+// Find a text field by label text, type it in, then blur the field.
+// Commonly used in our Admin section as we auto-save settings.
+export function typeAndBlurUsingLabel(label, value) {
+ cy.findByLabelText(label)
+ .clear()
+ .type(value)
+ .blur();
+}
+
Cypress.on("uncaught:exception", (err, runnable) => false);
diff --git a/frontend/test/cypress.json b/frontend/test/cypress.json
index e026f5171fd83b3223fb90ecc240632a80c71d1b..2d6709c27b82a6c323fee05e2a0e4c48b6149268 100644
--- a/frontend/test/cypress.json
+++ b/frontend/test/cypress.json
@@ -1,7 +1,7 @@
{
"testFiles": "**/*.cy.spec.js",
"pluginsFile": "frontend/test/cypress-plugins.js",
- "integrationFolder": "frontend/test",
+ "integrationFolder": "frontend/test/metabase",
"supportFile": "frontend/test/__support__/cypress.js",
"viewportHeight": 800,
"viewportWidth": 1280
diff --git a/frontend/test/metabase-db/mongo/add.cy.spec.js b/frontend/test/metabase-db/mongo/add.cy.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..dde062aa147700199c5f6587f46b2c7ea4521e5f
--- /dev/null
+++ b/frontend/test/metabase-db/mongo/add.cy.spec.js
@@ -0,0 +1,58 @@
+import {
+ signInAsAdmin,
+ restore,
+ modal,
+ typeAndBlurUsingLabel,
+} from "__support__/cypress";
+
+function addMongoDatabase() {
+ cy.visit("/admin/databases/create");
+ cy.contains("Database type")
+ .closest(".Form-field")
+ .find("a")
+ .click();
+ cy.contains("MongoDB").click({ force: true });
+ cy.contains("Additional Mongo connection");
+
+ typeAndBlurUsingLabel("Name", "MongoDB");
+ typeAndBlurUsingLabel("Database name", "admin");
+
+ cy.findByText("Save")
+ .should("not.be.disabled")
+ .click();
+}
+
+describe("mongodb > admin > add", () => {
+ beforeEach(() => {
+ restore();
+ signInAsAdmin();
+ cy.server();
+ });
+
+ it("should add a database and redirect to listing", () => {
+ cy.route({
+ method: "POST",
+ url: "/api/database",
+ }).as("createDatabase");
+
+ addMongoDatabase();
+
+ cy.wait("@createDatabase");
+
+ cy.url().should("match", /\/admin\/databases\?created=\d+$/);
+ cy.contains("Your database has been added!");
+ modal()
+ .contains("I'm good thanks")
+ .click();
+ });
+
+ it("can query a Mongo database", () => {
+ addMongoDatabase();
+ cy.url().should("match", /\/admin\/databases\?created=\d+$/);
+ cy.visit("/question/new");
+ cy.contains("Simple question").click();
+ cy.contains("MongoDB").click();
+ cy.contains("Version").click();
+ cy.contains("featureCompatibilityVersion");
+ });
+});
diff --git a/frontend/test/metabase-db/mongo/query.cy.spec.js b/frontend/test/metabase-db/mongo/query.cy.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..e156f3dfafa57654920c2809de94a0f1f7d792a7
--- /dev/null
+++ b/frontend/test/metabase-db/mongo/query.cy.spec.js
@@ -0,0 +1,110 @@
+import {
+ signInAsAdmin,
+ restore,
+ modal,
+ signInAsNormalUser,
+} from "__support__/cypress";
+
+function addMongoDatabase() {
+ cy.request("POST", "/api/database", {
+ engine: "mongo",
+ name: "MongoDB",
+ details: {
+ host: "localhost",
+ dbname: "admin",
+ port: 27017,
+ user: null,
+ pass: null,
+ authdb: null,
+ "additional-options": null,
+ "use-srv": false,
+ "tunnel-enabled": false,
+ },
+ auto_run_queries: true,
+ is_full_sync: true,
+ schedules: {
+ cache_field_values: {
+ schedule_day: null,
+ schedule_frame: null,
+ schedule_hour: 0,
+ schedule_type: "daily",
+ },
+ metadata_sync: {
+ schedule_day: null,
+ schedule_frame: null,
+ schedule_hour: null,
+ schedule_type: "hourly",
+ },
+ },
+ });
+}
+
+describe("mongodb > user > query", () => {
+ before(() => {
+ restore();
+ signInAsAdmin();
+ addMongoDatabase();
+ });
+
+ beforeEach(() => {
+ signInAsNormalUser();
+ });
+
+ it("can query a Mongo database as a user", () => {
+ cy.visit("/question/new");
+ cy.contains("Simple question").click();
+ cy.contains("MongoDB").click();
+ cy.contains("Version").click();
+ cy.contains("featureCompatibilityVersion");
+ });
+
+ it.only("can write a native MongoDB query", () => {
+ cy.visit("/question/new");
+ cy.contains("Native query").click();
+ cy.contains("MongoDB").click();
+
+ cy.get(".ace_content").type(`[ { $count: "Total" } ]`, {
+ parseSpecialCharSequences: false,
+ });
+ cy.get(".NativeQueryEditor .Icon-play").click();
+ cy.contains("1");
+ });
+
+ it("can save a native MongoDB query", () => {
+ cy.server();
+ cy.route("POST", "/api/card").as("createQuestion");
+
+ cy.visit("/question/new");
+ cy.contains("Native query").click();
+ cy.contains("MongoDB").click();
+
+ cy.get(".ace_content").type(`[ { $count: "Total" } ]`, {
+ parseSpecialCharSequences: false,
+ });
+ cy.get(".NativeQueryEditor .Icon-play").click();
+ cy.contains("1");
+
+ // Close the Ace editor because it interferes with the modal for some reason
+ cy.get(".Icon-contract").click();
+
+ cy.contains("Save").click();
+ modal()
+ .findByLabelText("Name")
+ .focus()
+ .type("mongo count");
+ modal()
+ .contains("button", "Save")
+ .should("not.be.disabled")
+ .click();
+
+ cy.wait("@createQuestion").then(({ status }) => {
+ expect(status).to.equal(202);
+ });
+
+ modal()
+ .contains("Not now")
+ .click();
+
+ cy.url().should("match", /\/question\/\d+$/);
+ });
+});
diff --git a/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js b/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js
index 734d0525fc07abee9a47545c1543bb16ab2c41d5..379fa29508211d402c52db223dfcb1799326ab79 100644
--- a/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js
+++ b/frontend/test/metabase-lib/lib/queries/structured/Filter.unit.spec.js
@@ -1,6 +1,6 @@
import Filter from "metabase-lib/lib/queries/structured/Filter";
-import { ORDERS } from "__support__/sample_dataset_fixture";
+import { ORDERS, PEOPLE } from "__support__/sample_dataset_fixture";
const query = ORDERS.query();
@@ -72,5 +72,38 @@ describe("Filter", () => {
filter(["segment", 1]).setDimension(["field-id", ORDERS.TOTAL.id]),
).toEqual([null, ["field-id", ORDERS.TOTAL.id]]);
});
+ it("should set joined-field for new filter clause", () => {
+ const q = ORDERS.query().join({
+ alias: "foo",
+ "source-table": PEOPLE.id,
+ });
+ const f = new Filter([], 0, q);
+ expect(
+ f.setDimension(["joined-field", "foo", ["field-id", PEOPLE.EMAIL.id]], {
+ useDefaultOperator: true,
+ }),
+ ).toEqual([
+ "=",
+ ["joined-field", "foo", ["field-id", PEOPLE.EMAIL.id]],
+ undefined,
+ ]);
+ });
});
+
+ const CASES = [
+ ["isStandard", ["=", ["field-id", 1]]],
+ ["isStandard", [null, ["field-id", 1]]], // assume null operator is standard
+ ["isSegment", ["segment", 1]],
+ ["isCustom", ["or", ["=", ["field-id", 1], 42]]],
+ ];
+ for (const method of ["isStandard", "isSegment", "isCustom"]) {
+ describe(method, () => {
+ for (const [method_, mbql] of CASES) {
+ const expected = method_ === method;
+ it(`should return ${expected} for ${JSON.stringify(mbql)}`, () => {
+ expect(filter(mbql)[method]()).toEqual(expected);
+ });
+ }
+ });
+ }
});
diff --git a/frontend/test/metabase/lib/expressions/suggest.unit.spec.js b/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
index 69366fd5c8bceebfd11cea26da351d6a59109f83..05fe56de44191687a8a173f832365c87229aac0c 100644
--- a/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
+++ b/frontend/test/metabase/lib/expressions/suggest.unit.spec.js
@@ -35,7 +35,7 @@ const STRING_FUNCTIONS = [
{ text: "ltrim(", type: "functions" },
{ text: "regexextract(", type: "functions" },
{ text: "rtrim(", type: "functions" },
- { text: "substitute(", type: "functions" },
+ { text: "replace(", type: "functions" },
{ text: "substring(", type: "functions" },
{ text: "trim(", type: "functions" },
{ text: "upper(", type: "functions" },
diff --git a/frontend/test/metabase/lib/query/field_ref.unit.spec.js b/frontend/test/metabase/lib/query/field_ref.unit.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..d58fd540f469d925e2aec4a53cdc545c64c5ac98
--- /dev/null
+++ b/frontend/test/metabase/lib/query/field_ref.unit.spec.js
@@ -0,0 +1,18 @@
+import { isValidField } from "metabase/lib/query/field_ref";
+
+describe("field_ref", () => {
+ describe("isValidField", () => {
+ it("should be valid for field id", () => {
+ expect(isValidField(["field-id", 1])).toBe(true);
+ });
+ it("should be valid for fk", () => {
+ expect(isValidField(["fk->", ["field-id", 1], ["field-id", 2]])).toBe(
+ true,
+ );
+ });
+ it("should be valid for joined-field", () => {
+ expect(isValidField(["joined-field", "foo", ["field-id", 1]])).toBe(true);
+ });
+ // TODO: remaininng field types
+ });
+});
diff --git a/frontend/test/metabase/scenarios/admin/settings/settings.cy.spec.js b/frontend/test/metabase/scenarios/admin/settings/settings.cy.spec.js
index 872628e12d896ad01ee18b84abcff4fc376414fb..8110fae1f78f0c78152323444b1dcc2b584bf7d5 100644
--- a/frontend/test/metabase/scenarios/admin/settings/settings.cy.spec.js
+++ b/frontend/test/metabase/scenarios/admin/settings/settings.cy.spec.js
@@ -59,4 +59,126 @@ describe("scenarios > admin > settings", () => {
openOrdersTable();
cy.contains(/^February 11, 2019, 9:40 PM$/);
});
+
+ describe(" > embedding settings", () => {
+ it("should validate a premium embedding token has a valid format", () => {
+ cy.server();
+ cy.route("PUT", "/api/setting/premium-embedding-token").as(
+ "saveEmbeddingToken",
+ );
+
+ cy.visit("/admin/settings/embedding_in_other_applications");
+ cy.contains("Premium embedding");
+ cy.contains("Enter a token").click();
+
+ // Try an invalid token format
+ cy.contains("Enter the token")
+ .next()
+ .type("Hi")
+ .blur();
+ cy.wait("@saveEmbeddingToken").then(({ response }) => {
+ expect(response.body).to.equal(
+ "Token format is invalid. Token should be 64 hexadecimal characters.",
+ );
+ });
+ cy.contains("Token format is invalid.");
+ });
+
+ it("should validate a premium embedding token exists", () => {
+ cy.server();
+ cy.route("PUT", "/api/setting/premium-embedding-token").as(
+ "saveEmbeddingToken",
+ );
+
+ cy.visit("/admin/settings/embedding_in_other_applications");
+ cy.contains("Premium embedding");
+ cy.contains("Enter a token").click();
+
+ // Try a valid format, but an invalid token
+ cy.contains("Enter the token")
+ .next()
+ .type(
+ "11397b1e60cfb1372f2f33ac8af234a15faee492bbf5c04d0edbad76da3e614a",
+ )
+ .blur();
+ cy.wait("@saveEmbeddingToken").then(({ response }) => {
+ expect(response.body).to.equal(
+ "Unable to validate token: 404 not found.",
+ );
+ });
+ cy.contains("Unable to validate token: 404 not found.");
+ });
+
+ it("should be able to set a premium embedding token", () => {
+ // A random embedding token with valid format
+ const embeddingToken =
+ "11397b1e60cfb1372f2f33ac8af234a15faee492bbf5c04d0edbad76da3e614a";
+
+ cy.server();
+ cy.route({
+ method: "PUT",
+ url: "/api/setting/premium-embedding-token",
+ response: embeddingToken,
+ }).as("saveEmbeddingToken");
+
+ cy.visit("/admin/settings/embedding_in_other_applications");
+ cy.contains("Premium embedding");
+ cy.contains("Enter a token").click();
+
+ cy.route("GET", "/api/session/properties").as("getSessionProperties");
+ cy.route({
+ method: "GET",
+ url: "/api/setting",
+ response: [
+ { key: "enable-embedding", value: true },
+ { key: "embedding-secret-key", value: embeddingToken },
+ { key: "premium-embedding-token", value: embeddingToken },
+ ],
+ }).as("getSettings");
+
+ cy.contains("Enter the token")
+ .next()
+ .type(embeddingToken)
+ .blur();
+ cy.wait("@saveEmbeddingToken").then(({ response }) => {
+ expect(response.body).to.equal(embeddingToken);
+ });
+ cy.wait("@getSessionProperties");
+ cy.wait("@getSettings");
+ cy.contains("Premium embedding enabled");
+ });
+ });
+
+ describe(" > email settings", () => {
+ it("should be able to save email settings", () => {
+ cy.visit("/admin/settings/email");
+ cy.findByPlaceholderText("smtp.yourservice.com")
+ .type("localhost")
+ .blur();
+ cy.findByPlaceholderText("587")
+ .type("1234")
+ .blur();
+ cy.findByPlaceholderText("metabase@yourcompany.com")
+ .type("admin@metabase.com")
+ .blur();
+ cy.findByText("Save changes").click();
+
+ cy.findByText("Changes saved!");
+ });
+ it("should show an error if test email fails", () => {
+ cy.visit("/admin/settings/email");
+ cy.findByText("Send test email").click();
+ cy.findByText("Sorry, something went wrong. Please try again.");
+ });
+ it("should be able to clear email settings", () => {
+ cy.visit("/admin/settings/email");
+ cy.findByText("Clear").click();
+ cy.findByPlaceholderText("smtp.yourservice.com").should("have.value", "");
+ cy.findByPlaceholderText("587").should("have.value", "");
+ cy.findByPlaceholderText("metabase@yourcompany.com").should(
+ "have.value",
+ "",
+ );
+ });
+ });
});
diff --git a/frontend/test/metabase/scenarios/question/native.cy.spec.js b/frontend/test/metabase/scenarios/question/native.cy.spec.js
index db852aee0cf46468cc60f3d5d5aeb85ee9442214..3a7654de2d43a966003d600b15b18b71bc2703b6 100644
--- a/frontend/test/metabase/scenarios/question/native.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/native.cy.spec.js
@@ -1,4 +1,9 @@
-import { signInAsNormalUser, restore, popover } from "__support__/cypress";
+import {
+ signInAsNormalUser,
+ restore,
+ popover,
+ modal,
+} from "__support__/cypress";
describe("scenarios > question > native", () => {
before(restore);
@@ -150,4 +155,39 @@ describe("scenarios > question > native", () => {
cy.get(".NativeQueryEditor .Icon-play").click();
cy.contains("18,760");
});
+
+ it("can load a question with a date filter (from issue metabase#12228)", () => {
+ cy.request("POST", "/api/card", {
+ name: "Test Question",
+ dataset_query: {
+ type: "native",
+ native: {
+ query: "select count(*) from orders where {{created_at}}",
+ "template-tags": {
+ created_at: {
+ id: "6b8b10ef-0104-1047-1e1b-2492d5954322",
+ name: "created_at",
+ "display-name": "Created at",
+ type: "dimension",
+ dimension: ["field-id", 15],
+ "widget-type": "date/month-year",
+ },
+ },
+ },
+ database: 1,
+ },
+ display: "scalar",
+ description: null,
+ visualization_settings: {},
+ collection_id: null,
+ result_metadata: null,
+ metadata_checksum: null,
+ }).then(response => {
+ cy.visit(`/question/${response.body.id}?created_at=2020-01`);
+ modal()
+ .contains("Okay")
+ .click();
+ cy.contains("580");
+ });
+ });
});
diff --git a/frontend/test/metabase/support/join_filter.cy.spec.js b/frontend/test/metabase/support/join_filter.cy.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..bb0ba3eb1967f29d873b15ce2a905b7cac9eb061
--- /dev/null
+++ b/frontend/test/metabase/support/join_filter.cy.spec.js
@@ -0,0 +1,26 @@
+import { restore, signInAsNormalUser } from "__support__/cypress";
+
+describe("support > join filter (metabase#12221)", () => {
+ before(restore);
+ before(signInAsNormalUser);
+
+ it.skip("can filter by a joined table", () => {
+ cy.visit("/question/new");
+ cy.contains("Custom question").click();
+ cy.contains("Sample Dataset").click();
+ cy.contains("Orders").click();
+ cy.get(".Icon-join_left_outer ").click();
+ cy.contains("People").click();
+ cy.contains("Orders + People");
+ cy.contains("Visualize").click();
+ cy.contains("Showing first 2,000");
+
+ cy.contains("Filter").click();
+ cy.contains("Email").click();
+ cy.contains("People – Email");
+ cy.get('[placeholder="Search by Email"]').type("wolf.");
+ cy.contains("wolf.dina@yahoo.com").click();
+ cy.contains("Add filter").click();
+ cy.contains("Showing 1 row");
+ });
+});
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo.clj b/modules/drivers/mongo/src/metabase/driver/mongo.clj
index f02a9a8b2326e7beaee6b48b2a2de3526e52b2d1..e1abacd7973446f0a96774e89821baea14451f80 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo.clj
@@ -207,12 +207,8 @@
(defmethod driver/execute-reducible-query :mongo
[_ query context respond]
- ;; TODO - there's a bug in normalization where the columns in `:projection` are getting converted to keywords
- ;; (#11897). Until we fix that, work around it here
- (let [query (cond-> query
- (-> query :native :projections seq) (update-in [:native :projections] (partial mapv u/qualified-name)))]
- (with-mongo-connection [_ (qp.store/database)]
- (execute/execute-reducible-query query context respond))))
+ (with-mongo-connection [_ (qp.store/database)]
+ (execute/execute-reducible-query query context respond)))
(defmethod driver/substitute-native-parameters :mongo
[driver inner-query]
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo/execute.clj b/modules/drivers/mongo/src/metabase/driver/mongo/execute.clj
index 8772fcb0d092e88ec69c6a9c9b68ebfc313a282d..7e4cbdea40b20a4e6bf83390be3da27956081fca 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo/execute.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo/execute.clj
@@ -49,7 +49,9 @@
not-in-expected (set/difference actual-cols expected-cols)]
(when (seq not-in-expected)
(throw (ex-info (tru "Unexpected columns in results: {0}" (sort not-in-expected))
- {:type error-type/driver}))))))
+ {:type error-type/driver
+ :actual actual-cols
+ :expected expected-cols}))))))
(s/defn ^:private result-col-names :- {:row [s/Str], :unescaped [s/Str]}
"Return column names we can expect in each `:row` of the results, and the `:unescaped` versions we should return in
@@ -68,7 +70,7 @@
(vec (for [k projections]
(get unescape-map k k))))})))
-(defn- result-metadata [{:keys [mbql?]} unescaped-col-names]
+(defn- result-metadata [unescaped-col-names]
{:cols (vec (for [col-name unescaped-col-names]
{:name col-name}))})
@@ -148,7 +150,7 @@
{row-col-names :row
unescaped-col-names :unescaped} (result-col-names native-query (row-keys first-row))]
(log/tracef "Renaming columns in results %s -> %s" (pr-str row-col-names) (pr-str unescaped-col-names))
- (respond (result-metadata native-query unescaped-col-names)
+ (respond (result-metadata unescaped-col-names)
(if-not first-row
[]
(reducible-rows context cursor first-row (post-process-row native-query row-col-names)))))
diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj
index 592cf16fe01639bcacc8964918d6bc4191bc3b3c..16884bd808beb5e4ae5d2918a1e7ba7f4396f93d 100644
--- a/src/metabase/public_settings.clj
+++ b/src/metabase/public_settings.clj
@@ -10,12 +10,16 @@
[metabase.models
[common :as common]
[setting :as setting :refer [defsetting]]]
+ metabase.public-settings.metastore
[metabase.util
[i18n :refer [available-locales-with-names deferred-tru set-locale trs tru]]
[password :as password]]
[toucan.db :as db])
(:import java.util.UUID))
+;; These modules register settings but are otherwise unused. They still must be imported.
+(comment metabase.public-settings.metastore/keep-me)
+
(defsetting check-for-updates
(deferred-tru "Identify when new versions of Metabase are available.")
:type :boolean
diff --git a/src/metabase/query_processor/middleware/catch_exceptions.clj b/src/metabase/query_processor/middleware/catch_exceptions.clj
index da10ec67e82df17ee3bc7939406a66dc0d2b1721..5c6522ec5c7e880f7c0c3b8b333f865572904a22 100644
--- a/src/metabase/query_processor/middleware/catch_exceptions.clj
+++ b/src/metabase/query_processor/middleware/catch_exceptions.clj
@@ -7,6 +7,7 @@
[reducible :as qp.reducible]]
[metabase.query-processor.middleware.permissions :as perms]
[metabase.util :as u]
+ [metabase.util.i18n :refer [trs]]
schema.utils)
(:import clojure.lang.ExceptionInfo
java.sql.SQLException
@@ -83,6 +84,8 @@
(assoc ((get-method format-exception Throwable) e)
:state (.getSQLState e)))
+;; TODO -- some of this logic duplicates the functionality of `clojure.core/Throwable->map`, we should consider
+;; whether we can use that more extensively and remove some of this logic
(defn- cause [^Throwable e]
(let [cause (.getCause e)]
(when-not (= cause e)
@@ -131,7 +134,9 @@
(defn- query-execution-info [query-execution]
(dissoc query-execution :result_rows :hash :executor_id :card_id :dashboard_id :pulse_id :native :start_time_millis))
-(defn- format-exception* [query ^Throwable e extra-info]
+(defn- format-exception*
+ "Format a `Throwable` into the usual userland error-response format."
+ [query ^Throwable e extra-info]
(try
(if-let [query-execution (:query-execution (ex-data e))]
(merge (query-execution-info query-execution)
@@ -147,7 +152,6 @@
"Middleware for catching exceptions thrown by the query processor and returning them in a 'normal' format. Forwards
exceptions to the `result-chan`."
[qp]
-
(fn [query rff {:keys [preprocessedf nativef raisef], :as context}]
(let [extra-info (atom nil)]
(letfn [(preprocessedf* [query context]
@@ -157,11 +161,15 @@
(swap! extra-info assoc :native query)
(nativef query context))
(raisef* [e context]
+ ;; if the exception is the special quit-early exception, forward this to our parent `raisef` exception
+ ;; handler, which has logic for handling that case
(if (qp.reducible/quit-result e)
(raisef e context)
- (do
- (log/tracef "raisef* got %s, returning formatted exception" (class e))
- (context/resultf (format-exception* query e @extra-info) context))))]
+ ;; otherwise format the Exception and return it
+ (let [formatted-exception (format-exception* query e @extra-info)]
+ (log/error (str (trs "Error processing query: {0}" (:error format-exception))
+ "\n" (u/pprint-to-str formatted-exception)))
+ (context/resultf formatted-exception context))))]
(try
(qp query rff (assoc context
:preprocessedf preprocessedf*
diff --git a/src/metabase/query_processor/reducible.clj b/src/metabase/query_processor/reducible.clj
index ccf4a0b96b43bb9bcf6a4b55b24b97d17d660fef..20fc217177326efb0e65847e371dbb333c4b5da1 100644
--- a/src/metabase/query_processor/reducible.clj
+++ b/src/metabase/query_processor/reducible.clj
@@ -47,6 +47,7 @@
(defn quit
"Create a special Exception that, when thrown or raised in the QP, will cause `result` to be returned directly.
+ Similar in concept to using `reduced` to stip reduction early.
(context/raisef (qp.reducible/quit :my-result) context)"
[result]
diff --git a/src/metabase/sync/analyze/query_results.clj b/src/metabase/sync/analyze/query_results.clj
index 8de0cf0f26bc65ca41bb5ef4b890502adb1e4f3a..750598dbb42b5b278f2002b5d11f275c95c8777f 100644
--- a/src/metabase/sync/analyze/query_results.clj
+++ b/src/metabase/sync/analyze/query_results.clj
@@ -42,9 +42,9 @@
(s/defn ^:private maybe-infer-special-type :- ResultColumnMetadata
"Infer the special type and add it to the result metadata. If the inferred special type is nil, don't override the
special type with a nil special type"
- [result-metadata col]
+ [col]
(update
- result-metadata
+ col
:special_type
(fn [original-value]
;; If we already know the special type, becouse it is stored, don't classify again, but try to refine special
@@ -53,14 +53,14 @@
(nil :type/Number) (classify-name/infer-special-type col)
original-value))))
-(s/defn ^:private stored-column-metadata->result-column-metadata :- ResultColumnMetadata
- "The metadata in the column of our resultsets come from the metadata we store in the `Field` associated with the
- column. It is cheapest and easiest to just use that. This function takes what it can from the column metadata to
- populate the ResultColumnMetadata"
+(s/defn ^:private col->ResultColumnMetadata :- ResultColumnMetadata
+ "Make sure a `column` as it comes back from a driver's initial results metadata matches the schema for valid results
+ column metadata, adding placeholder values and removing nil keys."
[column]
;; HACK - not sure why we don't have display_name yet in some cases
(merge
- {:display_name (:name column)}
+ {:base_type :type/*
+ :display_name (:name column)}
(u/select-non-nil-keys column [:name :display_name :description :base_type :special_type :unit :fingerprint])))
(defn insights-rf
@@ -70,9 +70,7 @@
[{:keys [cols]}]
(let [cols (for [col cols]
(try
- (-> col
- stored-column-metadata->result-column-metadata
- (maybe-infer-special-type col))
+ (maybe-infer-special-type (col->ResultColumnMetadata col))
(catch Throwable e
(log/error e (trs "Error generating insights for column:") col)
col)))]
diff --git a/test/metabase/query_processor/middleware/catch_exceptions_test.clj b/test/metabase/query_processor/middleware/catch_exceptions_test.clj
index f74bf2412baca8263ffef1bf9126a25a4062441d..4c3e13b56f3896a98bcef5a1bb93c3969bd8a1a3 100644
--- a/test/metabase/query_processor/middleware/catch_exceptions_test.clj
+++ b/test/metabase/query_processor/middleware/catch_exceptions_test.clj
@@ -29,7 +29,7 @@
(let [e1 (ex-info "1" {:level 1})
e2 (ex-info "2" {:level 2, :type error-type/qp} e1)
e3 (ex-info "3" {:level 3} e2)]
- (is (= {:status :failed,
+ (is (= {:status :failed
:class clojure.lang.ExceptionInfo
:error "1"
:stacktrace true
diff --git a/test/metabase/query_processor/middleware/results_metadata_test.clj b/test/metabase/query_processor/middleware/results_metadata_test.clj
index 559e98bc97fe33212018adfcb28bfe65dc9d0858..a99cd1fa30b81b49b2f821be16f684840ea4fe8e 100644
--- a/test/metabase/query_processor/middleware/results_metadata_test.clj
+++ b/test/metabase/query_processor/middleware/results_metadata_test.clj
@@ -2,6 +2,7 @@
(:require [clojure.test :refer :all]
[metabase
[query-processor :as qp]
+ [test :as mt]
[util :as u]]
[metabase.mbql.schema :as mbql.s]
[metabase.models
@@ -11,17 +12,17 @@
[permissions-group :as group]]
[metabase.query-processor.middleware.results-metadata :as results-metadata]
[metabase.query-processor.util :as qputil]
- [metabase.test
- [data :as data]
- [util :as tu]]
+ [metabase.sync.analyze.query-results :as qr]
[metabase.test.data.users :as users]
[metabase.test.mock.util :as mutil]
- [metabase.util.encryption :as encrypt]
- [toucan.db :as db]
- [toucan.util.test :as tt]))
+ [metabase.test.util :as tu]
+ [metabase.util
+ [encryption :as encrypt]
+ [schema :as su]]
+ [toucan.db :as db]))
(defn- native-query [sql]
- {:database (data/id)
+ {:database (mt/id)
:type :native
:native {:query sql}})
@@ -29,9 +30,9 @@
(db/select-one-field :result_metadata Card :id (u/get-id card)))
(defn- round-to-2-decimals
- "Defaults `tu/round-all-decimals` to 2 digits"
+ "Defaults `mt/round-all-decimals` to 2 digits"
[data]
- (tu/round-all-decimals 2 data))
+ (mt/round-all-decimals 2 data))
(def ^:private default-card-results
[{:name "ID", :display_name "ID", :base_type "type/BigInteger",
@@ -54,7 +55,7 @@
(deftest save-result-metadata-test
(testing "test that Card result metadata is saved after running a Card"
- (tt/with-temp Card [card]
+ (mt/with-temp Card [card]
(let [result (qp/process-userland-query
(assoc (native-query "SELECT ID, NAME, PRICE, CATEGORY_ID, LATITUDE, LONGITUDE FROM VENUES")
:info {:card-id (u/get-id card)
@@ -65,7 +66,7 @@
(-> card card-metadata round-to-2-decimals tu/round-fingerprint-cols)))))
(testing "check that using a Card as your source doesn't overwrite the results metadata..."
- (tt/with-temp Card [card {:dataset_query (native-query "SELECT * FROM VENUES")
+ (mt/with-temp Card [card {:dataset_query (native-query "SELECT * FROM VENUES")
:result_metadata [{:name "NAME", :display_name "Name", :base_type "type/Text"}]}]
(let [result (qp/process-userland-query {:database mbql.s/saved-questions-virtual-database-id
:type :query
@@ -76,7 +77,7 @@
(card-metadata card)))))
(testing "...even when running via the API endpoint"
- (tt/with-temp* [Collection [collection]
+ (mt/with-temp* [Collection [collection]
Card [card {:collection_id (u/get-id collection)
:dataset_query (native-query "SELECT * FROM VENUES")
:result_metadata [{:name "NAME", :display_name "Name", :base_type "type/Text"}]}]]
@@ -154,7 +155,7 @@
:columns (for [col default-card-results-native]
(-> col (update :special_type keyword) (update :base_type keyword)))}
(-> (qp/process-userland-query
- {:database (data/id)
+ {:database (mt/id)
:type :native
:native {:query "SELECT ID, NAME, PRICE, CATEGORY_ID, LATITUDE, LONGITUDE FROM VENUES"}})
(get-in [:data :results_metadata])
@@ -164,13 +165,13 @@
(deftest card-with-datetime-breakout-by-year-test
(testing "make sure that a Card where a DateTime column is broken out by year works the way we'd expect"
- (tt/with-temp Card [card]
+ (mt/with-temp Card [card]
(qp/process-userland-query
- {:database (data/id)
+ {:database (mt/id)
:type :query
- :query {:source-table (data/id :checkins)
+ :query {:source-table (mt/id :checkins)
:aggregation [[:count]]
- :breakout [[:datetime-field [:field-id (data/id :checkins :date)] :year]]}
+ :breakout [[:datetime-field [:field-id (mt/id :checkins :date)] :year]]}
:info {:card-id (u/get-id card)
:query-hash (qputil/query-hash {})}})
(is (= [{:base_type "type/DateTime"
@@ -191,3 +192,16 @@
card-metadata
round-to-2-decimals
tu/round-fingerprint-cols))))))
+
+(deftest valid-results-metadata-test
+ (mt/test-drivers (mt/normal-drivers)
+ (letfn [(results-metadata [query]
+ (-> (qp/process-query query) :data :results_metadata :columns))]
+ (testing "MBQL queries should come back with valid results metadata"
+
+ (is (schema= (su/non-empty qr/ResultsMetadata)
+ (results-metadata (mt/query venues)))))
+
+ (testing "Native queries should come back with valid results metadata (#12265)"
+ (is (schema= (su/non-empty qr/ResultsMetadata)
+ (results-metadata (-> (mt/mbql-query venues) qp/query->native mt/native-query))))))))
diff --git a/test/metabase/query_processor_test/case_test.clj b/test/metabase/query_processor_test/case_test.clj
index 87eb1fda537b3d4fe407eedba5f5e09ed96c89ab..87aa53739d14fade18c6dc145e3472b786160f5d 100644
--- a/test/metabase/query_processor_test/case_test.clj
+++ b/test/metabase/query_processor_test/case_test.clj
@@ -1,92 +1,85 @@
(ns metabase.query-processor-test.case-test
(:require [clojure.test :refer :all]
[metabase
- [query-processor-test :refer :all]
- [test :as mt]]
- [metabase.models
- [metric :refer [Metric]]
- [segment :refer [Segment]]]
- [metabase.test
- [data :as data]
- [util :as tu]]
- [toucan.util.test :as tt]))
+ [models :refer [Metric Segment]]
+ [test :as mt]]))
(defn- test-case
[expr]
(some->> (mt/run-mbql-query venues {:aggregation [expr]})
- rows
+ mt/rows
ffirst
double))
(deftest test-case-aggregations
(mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- (is (= 116.0 (test-case [:sum [:case [[[:< [:field-id (data/id :venues :price)] 2] 2]
- [[:< [:field-id (data/id :venues :price)] 4] 1]]]])))
+ (is (= 116.0 (test-case [:sum [:case [[[:< [:field-id (mt/id :venues :price)] 2] 2]
+ [[:< [:field-id (mt/id :venues :price)] 4] 1]]]])))
(testing "Can we use fields as values"
- (is (= 179.0 (test-case [:sum [:case [[[:< [:field-id (data/id :venues :price)] 2] [:field-id (data/id :venues :price)]]
- [[:< [:field-id (data/id :venues :price)] 4] [:field-id (data/id :venues :price)]]]]]))))
+ (is (= 179.0 (test-case [:sum [:case [[[:< [:field-id (mt/id :venues :price)] 2] [:field-id (mt/id :venues :price)]]
+ [[:< [:field-id (mt/id :venues :price)] 4] [:field-id (mt/id :venues :price)]]]]]))))
(testing "Test else clause"
- (is (= 122.0 (test-case [:sum [:case [[[:< [:field-id (data/id :venues :price)] 2] 2]]
+ (is (= 122.0 (test-case [:sum [:case [[[:< [:field-id (mt/id :venues :price)] 2] 2]]
{:default 1}]]))))
(testing "Test implicit else (= nil) clause"
;; Some DBs return 0 for sum of nulls.
- (is ((some-fn nil? zero?) (test-case [:sum [:case [[[:> [:field-id (data/id :venues :price)] 200] 2]]]]))))
+ (is ((some-fn nil? zero?) (test-case [:sum [:case [[[:> [:field-id (mt/id :venues :price)] 200] 2]]]]))))
(testing "Test complex filters"
(is (= 34.0 (test-case [:sum
- [:case [[[:and [:< [:field-id (data/id :venues :price)] 4]
- [:or [:starts-with [:field-id (data/id :venues :name)] "M"]
- [:ends-with [:field-id (data/id :venues :name)] "t"]]]
- [:field-id (data/id :venues :price)]]]]]))))
+ [:case [[[:and [:< [:field-id (mt/id :venues :price)] 4]
+ [:or [:starts-with [:field-id (mt/id :venues :name)] "M"]
+ [:ends-with [:field-id (mt/id :venues :name)] "t"]]]
+ [:field-id (mt/id :venues :price)]]]]]))))
(testing "Can we use segments in case"
- (tt/with-temp* [Segment [{segment-id :id} {:table_id (data/id :venues)
- :definition {:source-table (data/id :venues)
- :filter [:< [:field-id (data/id :venues :price)] 4]}}]]
- (is (= 179.0 (test-case [:sum [:case [[[:segment segment-id] [:field-id (data/id :venues :price)]]]]])))))
+ (mt/with-temp Segment [{segment-id :id} {:table_id (mt/id :venues)
+ :definition {:source-table (mt/id :venues)
+ :filter [:< [:field-id (mt/id :venues :price)] 4]}}]
+ (is (= 179.0 (test-case [:sum [:case [[[:segment segment-id] [:field-id (mt/id :venues :price)]]]]])))))
(testing "Can we use case in metric"
- (tt/with-temp* [Metric [{metric-id :id} {:table_id (data/id :venues)
- :definition {:source-table (data/id :venues)
- :aggregation [:sum
- [:case [[[:< [:field-id (data/id :venues :price)] 4]
- [:field-id (data/id :venues :price)]]]]]}}]]
+ (mt/with-temp Metric [{metric-id :id} {:table_id (mt/id :venues)
+ :definition {:source-table (mt/id :venues)
+ :aggregation [:sum
+ [:case [[[:< [:field-id (mt/id :venues :price)] 4]
+ [:field-id (mt/id :venues :price)]]]]]}}]
(is (= 179.0 (test-case [:metric metric-id])))))
(testing "Can we use case with breakout"
(is (= [[2 0.0]
[3 0.0]
[4 1.0]
[5 1.0]]
- (->> {:aggregation [[:sum [:case [[[:< [:field-id (data/id :venues :price)] 2] [:field-id (data/id :venues :price)]]]
+ (->> {:aggregation [[:sum [:case [[[:< [:field-id (mt/id :venues :price)] 2] [:field-id (mt/id :venues :price)]]]
{:default 0}]]]
- :breakout [[:field-id (data/id :venues :category_id)]]
+ :breakout [[:field-id (mt/id :venues :category_id)]]
:limit 4}
(mt/run-mbql-query venues)
- (tu/round-all-decimals 2)
- rows
+ (mt/round-all-decimals 2)
+ mt/rows
(map (fn [[k v]]
[(long k) (double v)]))))))))
(deftest test-case-aggregations+expressions
(mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations :expressions)
(testing "Can we use case in metric expressions"
- (is (= 90.5 (test-case [:+ [:/ [:sum [:case [[[:< [:field-id (data/id :venues :price)] 4] [:field-id (data/id :venues :price)]]]
+ (is (= 90.5 (test-case [:+ [:/ [:sum [:case [[[:< [:field-id (mt/id :venues :price)] 4] [:field-id (mt/id :venues :price)]]]
{:default 0}]] 2] 1]))))
(testing "Can use expressions as values"
- (is (= 194.5 (test-case [:sum [:case [[[:< [:field-id (data/id :venues :price)] 2] [:+ [:field-id (data/id :venues :price)] 1]]
- [[:< [:field-id (data/id :venues :price)] 4] [:+ [:/ [:field-id (data/id :venues :price)] 2] 1]]]]]))))))
+ (is (= 194.5 (test-case [:sum [:case [[[:< [:field-id (mt/id :venues :price)] 2] [:+ [:field-id (mt/id :venues :price)] 1]]
+ [[:< [:field-id (mt/id :venues :price)] 4] [:+ [:/ [:field-id (mt/id :venues :price)] 2] 1]]]]]))))))
(deftest test-case-normalization
(mt/test-drivers (mt/normal-drivers-with-feature :basic-aggregations)
- (is (= 116.0 (test-case ["sum" ["case" [[["<" ["field-id" (data/id :venues :price)] 2] 2]
- [["<" ["field-id" (data/id :venues :price)] 4] 1]] ]])))))
+ (is (= 116.0 (test-case ["sum" ["case" [[["<" ["field-id" (mt/id :venues :price)] 2] 2]
+ [["<" ["field-id" (mt/id :venues :price)] 4] 1]] ]])))))
(deftest test-case-expressions
(mt/test-drivers (mt/normal-drivers-with-feature :expressions)
(is (= [nil -2.0 -1.0]
- (->> {:expressions {"case_test" [:case [[[:< [:field-id (data/id :venues :price)] 2] -1.0]
- [[:< [:field-id (data/id :venues :price)] 3] -2.0]] ]}
+ (->> {:expressions {"case_test" [:case [[[:< [:field-id (mt/id :venues :price)] 2] -1.0]
+ [[:< [:field-id (mt/id :venues :price)] 3] -2.0]] ]}
:fields [[:expression "case_test"]]}
(mt/run-mbql-query venues)
- rows
+ mt/rows
(map (comp #(some-> % double) first))
distinct
sort)))))