diff --git a/client/app/components/EditParameterSettingsDialog.jsx b/client/app/components/EditParameterSettingsDialog.jsx index fbae2eb71d..504d550ff2 100644 --- a/client/app/components/EditParameterSettingsDialog.jsx +++ b/client/app/components/EditParameterSettingsDialog.jsx @@ -1,23 +1,22 @@ - -import { includes, words, capitalize, clone, isNull } from 'lodash'; -import React, { useState, useEffect } from 'react'; -import PropTypes from 'prop-types'; -import Checkbox from 'antd/lib/checkbox'; -import Modal from 'antd/lib/modal'; -import Form from 'antd/lib/form'; -import Button from 'antd/lib/button'; -import Select from 'antd/lib/select'; -import Input from 'antd/lib/input'; -import Divider from 'antd/lib/divider'; -import { wrap as wrapDialog, DialogPropType } from '@/components/DialogWrapper'; -import { QuerySelector } from '@/components/QuerySelector'; -import { Query } from '@/services/query'; +import { includes, words, capitalize, clone, isNull } from "lodash"; +import React, { useState, useEffect } from "react"; +import PropTypes from "prop-types"; +import Checkbox from "antd/lib/checkbox"; +import Modal from "antd/lib/modal"; +import Form from "antd/lib/form"; +import Button from "antd/lib/button"; +import Select from "antd/lib/select"; +import Input from "antd/lib/input"; +import Divider from "antd/lib/divider"; +import { wrap as wrapDialog, DialogPropType } from "@/components/DialogWrapper"; +import { QuerySelector } from "@/components/QuerySelector"; +import { Query } from "@/services/query"; const { Option } = Select; const formItemProps = { labelCol: { span: 6 }, wrapperCol: { span: 16 } }; function getDefaultTitle(text) { - return capitalize(words(text).join(' ')); // humanize + return capitalize(words(text).join(" ")); // humanize } function isTypeDateRange(type) { @@ -26,28 +25,28 @@ function isTypeDateRange(type) { function joinExampleList(multiValuesOptions) { const { prefix, suffix } = multiValuesOptions; - return ['value1', 'value2', 'value3'] - .map(value => `${prefix}${value}${suffix}`) - .join(','); + return ["value1", "value2", "value3"] + .map((value) => `${prefix}${value}${suffix}`) + .join(","); } function NameInput({ name, type, onChange, existingNames, setValidation }) { - let helpText = ''; - let validateStatus = ''; + let helpText = ""; + let validateStatus = ""; if (!name) { - helpText = 'Choose a keyword for this parameter'; + helpText = "Choose a keyword for this parameter"; setValidation(false); } else if (includes(existingNames, name)) { - helpText = 'Parameter with this name already exists'; + helpText = "Parameter with this name already exists"; setValidation(false); - validateStatus = 'error'; + validateStatus = "error"; } else { if (isTypeDateRange(type)) { helpText = ( - Appears in query as {' '} - + Appears in query as{" "} + {`{{${name}.start}} {{${name}.end}}`} @@ -64,7 +63,7 @@ function NameInput({ name, type, onChange, existingNames, setValidation }) { validateStatus={validateStatus} {...formItemProps} > - onChange(e.target.value)} autoFocus /> + onChange(e.target.value)} autoFocus /> ); } @@ -101,12 +100,12 @@ function EditParameterSettingsDialog(props) { } // title - if (param.title === '') { + if (param.title === "") { return false; } // query - if (param.type === 'query' && !param.queryId) { + if (param.type === "query" && !param.queryId) { return false; } @@ -129,21 +128,29 @@ function EditParameterSettingsDialog(props) { return ( Cancel - ), ( - - )]} + footer={[ + , + , + ]} >
{isNew && ( setParam({ ...param, name })} + onChange={(name) => setParam({ ...param, name })} setValidation={setIsNameValid} existingNames={props.existingParams} type={param.type} @@ -151,91 +158,144 @@ function EditParameterSettingsDialog(props) { )} setParam({ ...param, title: e.target.value })} + value={ + isNull(param.title) ? getDefaultTitle(param.name) : param.title + } + onChange={(e) => setParam({ ...param, title: e.target.value })} data-test="ParameterTitleInput" /> - setParam({ ...param, type })} + data-test="ParameterTypeSelect" + > + + - - - + + + - + - + - {param.type === 'enum' && ( - + {param.type === "enum" && ( + setParam({ ...param, enumOptions: e.target.value })} + onChange={(e) => + setParam({ ...param, enumOptions: e.target.value }) + } /> )} - {param.type === 'query' && ( - + {param.type === "query" && ( + setParam({ ...param, queryId: q && q.id })} + onChange={(q) => setParam({ ...param, queryId: q && q.id })} type="select" /> )} - {(param.type === 'enum' || param.type === 'query') && ( - + {(param.type === "enum" || param.type === "query") && ( + setParam({ ...param, - multiValuesOptions: e.target.checked ? { - prefix: '', - suffix: '', - separator: ',', - } : null })} + onChange={(e) => + setParam({ + ...param, + multiValuesOptions: e.target.checked + ? { + prefix: "", + suffix: "", + separator: ",", + } + : null, + }) + } data-test="AllowMultipleValuesCheckbox" > - Allow multiple values + Allow multiple values )} - {(param.type === 'enum' || param.type === 'query') && param.multiValuesOptions && ( - - Placed in query as: {joinExampleList(param.multiValuesOptions)} - - )} - {...formItemProps} - > - - - )} + + + )}
); diff --git a/client/app/components/ParameterValueInput.jsx b/client/app/components/ParameterValueInput.jsx index 7f28ad33a8..8ad32139fa 100644 --- a/client/app/components/ParameterValueInput.jsx +++ b/client/app/components/ParameterValueInput.jsx @@ -1,21 +1,21 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import Select from 'antd/lib/select'; -import Input from 'antd/lib/input'; -import InputNumber from 'antd/lib/input-number'; -import DateParameter from '@/components/dynamic-parameters/DateParameter'; -import DateRangeParameter from '@/components/dynamic-parameters/DateRangeParameter'; -import { isEqual } from 'lodash'; -import { QueryBasedParameterInput } from './QueryBasedParameterInput'; - -import './ParameterValueInput.less'; +import React from "react"; +import PropTypes from "prop-types"; +import Select from "antd/lib/select"; +import Input from "antd/lib/input"; +import InputNumber from "antd/lib/input-number"; +import DateParameter from "@/components/dynamic-parameters/DateParameter"; +import DateRangeParameter from "@/components/dynamic-parameters/DateRangeParameter"; +import { isEqual, trim } from "lodash"; +import { QueryBasedParameterInput } from "./QueryBasedParameterInput"; + +import "./ParameterValueInput.less"; const { Option } = Select; const multipleValuesProps = { maxTagCount: 3, maxTagTextLength: 10, - maxTagPlaceholder: num => `+${num.length} more`, + maxTagPlaceholder: (num) => `+${num.length} more`, }; class ParameterValueInput extends React.Component { @@ -30,19 +30,21 @@ class ParameterValueInput extends React.Component { }; static defaultProps = { - type: 'text', + type: "text", value: null, - enumOptions: '', + enumOptions: "", queryId: null, parameter: null, onSelect: () => {}, - className: '', + className: "", }; constructor(props) { super(props); this.state = { - value: props.parameter.hasPendingValue ? props.parameter.pendingValue : props.value, + value: props.parameter.hasPendingValue + ? props.parameter.pendingValue + : props.value, isDirty: props.parameter.hasPendingValue, }; } @@ -56,13 +58,13 @@ class ParameterValueInput extends React.Component { isDirty: parameter.hasPendingValue, }); } - } + }; onSelect = (value) => { - const isDirty = !isEqual(value, this.props.value); + const isDirty = !isEqual(trim(value), trim(this.props.value)); this.setState({ value, isDirty }); this.props.onSelect(value, isDirty); - } + }; renderDateParameter() { const { type, parameter } = this.props; @@ -95,13 +97,14 @@ class ParameterValueInput extends React.Component { renderEnumInput() { const { enumOptions, parameter } = this.props; const { value } = this.state; - const enumOptionsArray = enumOptions.split('\n').filter(v => v !== ''); + const enumOptionsArray = enumOptions.split("\n").filter((v) => v !== ""); // Antd Select doesn't handle null in multiple mode - const normalize = val => (parameter.multiValuesOptions && val === null ? [] : val); + const normalize = (val) => + parameter.multiValuesOptions && val === null ? [] : val; return ( ); } @@ -124,7 +131,7 @@ class ParameterValueInput extends React.Component { return ( (isNaN(val) ? undefined : val); - return ( this.onSelect(normalize(val))} + value={value} + onChange={(val) => this.onSelect(val)} /> ); } @@ -160,7 +165,7 @@ class ParameterValueInput extends React.Component { className={className} value={value} data-test="TextParamInput" - onChange={e => this.onSelect(e.target.value)} + onChange={(e) => this.onSelect(e.target.value)} /> ); } @@ -168,16 +173,22 @@ class ParameterValueInput extends React.Component { renderInput() { const { type } = this.props; switch (type) { - case 'datetime-with-seconds': - case 'datetime-local': - case 'date': return this.renderDateParameter(); - case 'datetime-range-with-seconds': - case 'datetime-range': - case 'date-range': return this.renderDateRangeParameter(); - case 'enum': return this.renderEnumInput(); - case 'query': return this.renderQueryBasedInput(); - case 'number': return this.renderNumberInput(); - default: return this.renderTextInput(); + case "datetime-with-seconds": + case "datetime-local": + case "date": + return this.renderDateParameter(); + case "datetime-range-with-seconds": + case "datetime-range": + case "date-range": + return this.renderDateRangeParameter(); + case "enum": + return this.renderEnumInput(); + case "query": + return this.renderQueryBasedInput(); + case "number": + return this.renderNumberInput(); + default: + return this.renderTextInput(); } } @@ -185,7 +196,11 @@ class ParameterValueInput extends React.Component { const { isDirty } = this.state; return ( -
+
{this.renderInput()}
); diff --git a/client/app/components/Parameters.jsx b/client/app/components/Parameters.jsx index b77c47401c..e3a488695f 100644 --- a/client/app/components/Parameters.jsx +++ b/client/app/components/Parameters.jsx @@ -1,23 +1,31 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { size, filter, forEach, extend } from 'lodash'; -import { react2angular } from 'react2angular'; -import { SortableContainer, SortableElement, DragHandle } from '@/components/sortable'; -import { $location } from '@/services/ng'; -import { Parameter } from '@/services/parameters'; -import ParameterApplyButton from '@/components/ParameterApplyButton'; -import ParameterValueInput from '@/components/ParameterValueInput'; -import EditParameterSettingsDialog from './EditParameterSettingsDialog'; -import { toHuman } from '@/filters'; - -import './Parameters.less'; +import React from "react"; +import PropTypes from "prop-types"; +import { size, filter, forEach, extend, get, includes } from "lodash"; +import { react2angular } from "react2angular"; +import { + SortableContainer, + SortableElement, + DragHandle, +} from "@/components/sortable"; +import { $location } from "@/services/ng"; +import { Parameter } from "@/services/parameters"; +import ParameterApplyButton from "@/components/ParameterApplyButton"; +import ParameterValueInput from "@/components/ParameterValueInput"; +import Form from "antd/lib/form"; +import Tooltip from "antd/lib/tooltip"; +import EditParameterSettingsDialog from "./EditParameterSettingsDialog"; +import { toHuman } from "@/filters"; + +import "./Parameters.less"; function updateUrl(parameters) { const params = extend({}, $location.search()); parameters.forEach((param) => { extend(params, param.toUrlParams()); }); - Object.keys(params).forEach(key => params[key] == null && delete params[key]); + Object.keys(params).forEach( + (key) => params[key] == null && delete params[key] + ); $location.search(params); } @@ -29,6 +37,10 @@ export class Parameters extends React.Component { onValuesChange: PropTypes.func, onPendingValuesChange: PropTypes.func, onParametersEdit: PropTypes.func, + queryResultErrorData: PropTypes.shape({ + parameters: PropTypes.objectOf(PropTypes.string), + }), + unsavedParameters: PropTypes.arrayOf(PropTypes.string), }; static defaultProps = { @@ -38,25 +50,36 @@ export class Parameters extends React.Component { onValuesChange: () => {}, onPendingValuesChange: () => {}, onParametersEdit: () => {}, + queryResultErrorData: {}, + unsavedParameters: null, }; constructor(props) { super(props); const { parameters } = props; - this.state = { parameters }; + this.state = { + parameters, + touched: {}, + }; + if (!props.disableUrlUpdate) { updateUrl(parameters); } } componentDidUpdate = (prevProps) => { - const { parameters, disableUrlUpdate } = this.props; + const { parameters, disableUrlUpdate, queryResultErrorData } = this.props; if (prevProps.parameters !== parameters) { this.setState({ parameters }); if (!disableUrlUpdate) { updateUrl(parameters); } } + + // reset touched flags on new error data + if (prevProps.queryResultErrorData !== queryResultErrorData) { + this.setState({ touched: {} }); + } }; handleKeyDown = (e) => { @@ -69,14 +92,15 @@ export class Parameters extends React.Component { setPendingValue = (param, value, isDirty) => { const { onPendingValuesChange } = this.props; - this.setState(({ parameters }) => { + this.setState(({ parameters, touched }) => { if (isDirty) { param.setPendingValue(value); + touched = { ...touched, [param.name]: true }; } else { param.clearPendingValue(); } onPendingValuesChange(); - return { parameters }; + return { parameters, touched }; }); }; @@ -94,8 +118,10 @@ export class Parameters extends React.Component { applyChanges = () => { const { onValuesChange, disableUrlUpdate } = this.props; this.setState(({ parameters }) => { - const parametersWithPendingValues = parameters.filter(p => p.hasPendingValue); - forEach(parameters, p => p.applyPendingValue()); + const parametersWithPendingValues = parameters.filter( + (p) => p.hasPendingValue + ); + forEach(parameters, (p) => p.applyPendingValue()); if (!disableUrlUpdate) { updateUrl(parameters); } @@ -106,20 +132,53 @@ export class Parameters extends React.Component { showParameterSettings = (parameter, index) => { const { onParametersEdit } = this.props; - EditParameterSettingsDialog - .showModal({ parameter }) - .result.then((updated) => { - this.setState(({ parameters }) => { + EditParameterSettingsDialog.showModal({ parameter }).result.then( + (updated) => { + this.setState(({ parameters, touched }) => { + touched = { ...touched, [parameter.name]: true }; const updatedParameter = extend(parameter, updated); - parameters[index] = Parameter.create(updatedParameter, updatedParameter.parentQueryId); + parameters[index] = Parameter.create( + updatedParameter, + updatedParameter.parentQueryId + ); onParametersEdit(); - return { parameters }; + return { parameters, touched }; }); - }); + } + ); + }; + + getParameterFeedback = (param) => { + // error msg + const { queryResultErrorData } = this.props; + const error = get(queryResultErrorData, ["parameters", param.name], false); + if (error) { + const feedback = {error}; + return [feedback, "error"]; + } + + // unsaved + const { unsavedParameters } = this.props; + if (includes(unsavedParameters, param.name)) { + const feedback = ( + <> + Unsaved{" "} + + + + + ); + return [feedback, "warning"]; + } + + return []; }; renderParameter(param, index) { const { editable } = this.props; + const touched = this.state.touched[param.name]; + const [feedback, status] = this.getParameterFeedback(param); + return (
)}
- this.setPendingValue(param, value, isDirty)} - /> + + + this.setPendingValue(param, value, isDirty) + } + /> +
); } @@ -154,7 +220,7 @@ export class Parameters extends React.Component { render() { const { parameters } = this.state; const { editable } = this.props; - const dirtyParamCount = size(filter(parameters, 'hasPendingValue')); + const dirtyParamCount = size(filter(parameters, "hasPendingValue")); return ( {parameters.map((param, index) => (
- {editable && } + {editable && ( + + )} {this.renderParameter(param, index)}
))} - +
); } } export default function init(ngModule) { - ngModule.component('parameters', react2angular(Parameters)); + ngModule.component("parameters", react2angular(Parameters)); } init.init = true; diff --git a/client/app/components/Parameters.less b/client/app/components/Parameters.less index 338912fe80..ed94a4cd19 100644 --- a/client/app/components/Parameters.less +++ b/client/app/components/Parameters.less @@ -3,7 +3,7 @@ .parameter-block { display: inline-block; background: white; - padding: 0 12px 6px 0; + padding: 0 12px 17px 0; vertical-align: top; z-index: 1; @@ -15,12 +15,31 @@ .parameter-container.sortable-container & { margin: 4px 0 0 4px; - padding: 3px 6px 6px; + padding: 3px 6px 19px; } &.parameter-dragged { box-shadow: 0 4px 9px -3px rgba(102, 136, 153, 0.15); } + + .ant-form-item { + margin-bottom: 0 !important; + } + + .ant-form-explain { + position: absolute; + left: 0; + right: 0; + bottom: -20px; + font-size: 12px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + .ant-form-item-control { + line-height: normal; + } } .parameter-heading { diff --git a/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx b/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx index 3a4e31245b..4b18b4a021 100644 --- a/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx +++ b/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx @@ -1,61 +1,82 @@ -import React, { useState } from 'react'; -import PropTypes from 'prop-types'; -import { compact, isEmpty, invoke } from 'lodash'; -import { markdown } from 'markdown'; -import cx from 'classnames'; -import Menu from 'antd/lib/menu'; -import { currentUser } from '@/services/auth'; -import recordEvent from '@/services/recordEvent'; -import { formatDateTime } from '@/filters/datetime'; -import HtmlContent from '@/components/HtmlContent'; -import { Parameters } from '@/components/Parameters'; -import { TimeAgo } from '@/components/TimeAgo'; -import { Timer } from '@/components/Timer'; -import { Moment } from '@/components/proptypes'; -import QueryLink from '@/components/QueryLink'; -import { FiltersType } from '@/components/Filters'; -import ExpandedWidgetDialog from '@/components/dashboards/ExpandedWidgetDialog'; -import EditParameterMappingsDialog from '@/components/dashboards/EditParameterMappingsDialog'; -import { VisualizationRenderer } from '@/visualizations/VisualizationRenderer'; -import Widget from './Widget'; +import React, { useState } from "react"; +import PropTypes from "prop-types"; +import { compact, isEmpty, invoke } from "lodash"; +import { markdown } from "markdown"; +import cx from "classnames"; +import Menu from "antd/lib/menu"; +import { currentUser } from "@/services/auth"; +import recordEvent from "@/services/recordEvent"; +import { formatDateTime } from "@/filters/datetime"; +import HtmlContent from "@/components/HtmlContent"; +import { Parameters } from "@/components/Parameters"; +import { TimeAgo } from "@/components/TimeAgo"; +import { Timer } from "@/components/Timer"; +import { Moment } from "@/components/proptypes"; +import QueryLink from "@/components/QueryLink"; +import { FiltersType } from "@/components/Filters"; +import ExpandedWidgetDialog from "@/components/dashboards/ExpandedWidgetDialog"; +import EditParameterMappingsDialog from "@/components/dashboards/EditParameterMappingsDialog"; +import { VisualizationRenderer } from "@/visualizations/VisualizationRenderer"; +import Widget from "./Widget"; -function visualizationWidgetMenuOptions({ widget, canEditDashboard, onParametersEdit }) { - const canViewQuery = currentUser.hasPermission('view_query'); - const canEditParameters = canEditDashboard && !isEmpty(invoke(widget, 'query.getParametersDefs')); +function visualizationWidgetMenuOptions({ + widget, + canEditDashboard, + onParametersEdit, +}) { + const canViewQuery = currentUser.hasPermission("view_query"); + const canEditParameters = + canEditDashboard && !isEmpty(invoke(widget, "query.getParametersDefs")); const widgetQueryResult = widget.getQueryResult(); - const isQueryResultEmpty = !widgetQueryResult || !widgetQueryResult.isEmpty || widgetQueryResult.isEmpty(); + const isQueryResultEmpty = + !widgetQueryResult || + !widgetQueryResult.isEmpty || + widgetQueryResult.isEmpty(); - const downloadLink = fileType => widgetQueryResult.getLink(widget.getQuery().id, fileType); - const downloadName = fileType => widgetQueryResult.getName(widget.getQuery().name, fileType); + const downloadLink = (fileType) => + widgetQueryResult.getLink(widget.getQuery().id, fileType); + const downloadName = (fileType) => + widgetQueryResult.getName(widget.getQuery().name, fileType); return compact([ {!isQueryResultEmpty ? ( - + Download as CSV File - ) : 'Download as CSV File'} + ) : ( + "Download as CSV File" + )} , {!isQueryResultEmpty ? ( - + Download as Excel File - ) : 'Download as Excel File'} + ) : ( + "Download as Excel File" + )} , - ((canViewQuery || canEditParameters) && ), + (canViewQuery || canEditParameters) && , canViewQuery && ( - View Query + + View Query + ), - (canEditParameters && ( - + canEditParameters && ( + Edit Parameters - )), + ), ]); } @@ -73,8 +94,15 @@ function RefreshIndicator({ refreshStartedAt }) { RefreshIndicator.propTypes = { refreshStartedAt: Moment }; RefreshIndicator.defaultProps = { refreshStartedAt: null }; -function VisualizationWidgetHeader({ widget, refreshStartedAt, parameters, onParametersUpdate }) { - const canViewQuery = currentUser.hasPermission('view_query'); +function VisualizationWidgetHeader({ + widget, + refreshStartedAt, + parameters, + onParametersUpdate, +}) { + const canViewQuery = currentUser.hasPermission("view_query"); + const queryResult = widget.getQueryResult(); + const errorData = queryResult && queryResult.getErrorData(); return ( <> @@ -82,16 +110,24 @@ function VisualizationWidgetHeader({ widget, refreshStartedAt, parameters, onPar

- +

- {markdown.toHTML(widget.getQuery().description || '')} + {markdown.toHTML(widget.getQuery().description || "")}
{!isEmpty(parameters) && ( -
- +
+
)} @@ -113,7 +149,7 @@ VisualizationWidgetHeader.defaultProps = { function VisualizationWidgetFooter({ widget, isPublic, onRefresh, onExpand }) { const widgetQueryResult = widget.getQueryResult(); - const updatedAt = invoke(widgetQueryResult, 'getUpdatedAt'); + const updatedAt = invoke(widgetQueryResult, "getUpdatedAt"); const [refreshClickButtonId, setRefreshClickButtonId] = useState(); const refreshWidget = (buttonId) => { @@ -126,22 +162,27 @@ function VisualizationWidgetFooter({ widget, isPublic, onRefresh, onExpand }) { return ( <> - {(!isPublic && !!widgetQueryResult) && ( + {!isPublic && !!widgetQueryResult && ( refreshWidget(1)} data-test="RefreshButton" > - {' '} + {" "} )} - {' '}{formatDateTime(updatedAt)} + {formatDateTime(updatedAt)} {isPublic && ( - {' '} + {" "} + )} @@ -151,7 +192,11 @@ function VisualizationWidgetFooter({ widget, isPublic, onRefresh, onExpand }) { className="btn btn-sm btn-default hidden-print btn-transparent btn__refresh" onClick={() => refreshWidget(2)} > - + )} { - const { widget, dashboard, onRefresh, onParameterMappingsChange } = this.props; + const { widget, dashboard, onRefresh, onParameterMappingsChange } = + this.props; EditParameterMappingsDialog.showModal({ dashboard, widget, @@ -233,17 +283,18 @@ class VisualizationWidget extends React.Component { const widgetQueryResult = widget.getQueryResult(); const widgetStatus = widgetQueryResult && widgetQueryResult.getStatus(); switch (widgetStatus) { - case 'failed': + case "failed": return (
{widgetQueryResult.getError() && (
- Error running query: {widgetQueryResult.getError()} + Error running query:{" "} + {widgetQueryResult.getError()}
)}
); - case 'done': + case "done": return (
- )} - footer={( + } + footer={ - )} - tileProps={{ 'data-refreshing': isRefreshing }} + } + tileProps={{ "data-refreshing": isRefreshing }} > {this.renderVisualization()} diff --git a/client/app/components/queries/visualization-embed.html b/client/app/components/queries/visualization-embed.html index 563e4021e5..44bb137a29 100644 --- a/client/app/components/queries/visualization-embed.html +++ b/client/app/components/queries/visualization-embed.html @@ -13,7 +13,7 @@

- +
diff --git a/client/app/components/queries/visualization-embed.js b/client/app/components/queries/visualization-embed.js index fd3fa1c7f0..28dbd420a3 100644 --- a/client/app/components/queries/visualization-embed.js +++ b/client/app/components/queries/visualization-embed.js @@ -1,19 +1,21 @@ -import { find } from 'lodash'; -import moment from 'moment'; -import logoUrl from '@/assets/images/redash_icon_small.png'; -import template from './visualization-embed.html'; +import logoUrl from "@/assets/images/redash_icon_small.png"; +import { find } from "lodash"; +import moment from "moment"; + +import template from "./visualization-embed.html"; const VisualizationEmbed = { template, bindings: { - query: '<', + query: "<", }, controller($routeParams) { - 'ngInject'; + "ngInject"; this.refreshQueryResults = () => { this.loading = true; this.error = null; + this.errorData = {}; this.refreshStartedAt = moment(); this.query .getQueryResultPromise() @@ -24,11 +26,15 @@ const VisualizationEmbed = { .catch((error) => { this.loading = false; this.error = error.getError(); + this.errorData = error.getErrorData(); }); }; const visualizationId = parseInt($routeParams.visualizationId, 10); - this.visualization = find(this.query.visualizations, visualization => visualization.id === visualizationId); + this.visualization = find( + this.query.visualizations, + (visualization) => visualization.id === visualizationId + ); this.showQueryDescription = $routeParams.showDescription; this.logoUrl = logoUrl; this.apiKey = $routeParams.api_key; @@ -37,14 +43,14 @@ const VisualizationEmbed = { this.hideHeader = $routeParams.hide_header !== undefined; this.hideQueryLink = $routeParams.hide_link !== undefined; - document.querySelector('body').classList.add('headless'); + document.querySelector("body").classList.add("headless"); this.refreshQueryResults(); }, }; export default function init(ngModule) { - ngModule.component('visualizationEmbed', VisualizationEmbed); + ngModule.component("visualizationEmbed", VisualizationEmbed); function loadSession($route, Auth) { const apiKey = $route.current.params.api_key; @@ -53,19 +59,25 @@ export default function init(ngModule) { } function loadQuery($route, Auth, Query) { - 'ngInject'; + "ngInject"; - return loadSession($route, Auth).then(() => Query.get({ id: $route.current.params.queryId }).$promise); + return loadSession($route, Auth).then( + () => Query.get({ id: $route.current.params.queryId }).$promise + ); } ngModule.config(($routeProvider) => { - $routeProvider.when('/embed/query/:queryId/visualization/:visualizationId', { - resolve: { - query: loadQuery, - }, - reloadOnSearch: false, - template: '', - }); + $routeProvider.when( + "/embed/query/:queryId/visualization/:visualizationId", + { + resolve: { + query: loadQuery, + }, + reloadOnSearch: false, + template: + '', + } + ); }); } diff --git a/client/app/pages/dashboards/dashboard.html b/client/app/pages/dashboards/dashboard.html index 114a07fd63..3c8bd896e4 100644 --- a/client/app/pages/dashboards/dashboard.html +++ b/client/app/pages/dashboards/dashboard.html @@ -93,8 +93,8 @@

-
- +
+
diff --git a/client/app/pages/dashboards/public-dashboard-page.html b/client/app/pages/dashboards/public-dashboard-page.html index 2c1d251d3f..7246e8d86f 100644 --- a/client/app/pages/dashboards/public-dashboard-page.html +++ b/client/app/pages/dashboards/public-dashboard-page.html @@ -1,8 +1,8 @@
-
- +
+
diff --git a/client/app/pages/queries/query.html b/client/app/pages/queries/query.html index 15ecb22e33..34855c792f 100644 --- a/client/app/pages/queries/query.html +++ b/client/app/pages/queries/query.html @@ -199,8 +199,8 @@

- +
diff --git a/client/app/pages/queries/source-view.js b/client/app/pages/queries/source-view.js index f5d72898b1..ec5755c502 100644 --- a/client/app/pages/queries/source-view.js +++ b/client/app/pages/queries/source-view.js @@ -1,6 +1,7 @@ -import { map, debounce } from 'lodash'; -import template from './query.html'; -import EditParameterSettingsDialog from '@/components/EditParameterSettingsDialog'; +import { debounce, isEmpty, isEqual, map } from "lodash"; + +import template from "./query.html"; +import EditParameterSettingsDialog from "@/components/EditParameterSettingsDialog"; function QuerySourceCtrl( Events, @@ -10,12 +11,12 @@ function QuerySourceCtrl( $uibModal, currentUser, KeyboardShortcuts, - $rootScope, + $rootScope ) { // extends QueryViewCtrl - $controller('QueryViewCtrl', { $scope }); + $controller("QueryViewCtrl", { $scope }); - Events.record('view_source', 'query', $scope.query.id); + Events.record("view_source", "query", $scope.query.id); const isNewQuery = !$scope.query.id; let queryText = $scope.query.query; @@ -27,35 +28,36 @@ function QuerySourceCtrl( $scope.modKey = KeyboardShortcuts.modKey; // @override - Object.defineProperty($scope, 'showDataset', { + Object.defineProperty($scope, "showDataset", { get() { - return $scope.queryResult && $scope.queryResult.getStatus() === 'done'; + return $scope.queryResult && $scope.queryResult.getStatus() === "done"; }, }); const shortcuts = { - 'mod+s': function save() { + "mod+s": function save() { if ($scope.canEdit) { $scope.saveQuery(); } }, - 'mod+p': () => { + "mod+p": () => { $scope.addNewParameter(); }, }; KeyboardShortcuts.bind(shortcuts); - $scope.$on('$destroy', () => { + $scope.$on("$destroy", () => { KeyboardShortcuts.unbind(shortcuts); }); - $scope.canForkQuery = () => currentUser.hasPermission('edit_query') && !$scope.dataSource.view_only; + $scope.canForkQuery = () => + currentUser.hasPermission("edit_query") && !$scope.dataSource.view_only; - $scope.updateQuery = debounce( - newQueryText => $scope.$apply(() => { + $scope.updateQuery = debounce((newQueryText) => + $scope.$apply(() => { $scope.query.query = newQueryText; - }), + }) ); // @override @@ -78,21 +80,23 @@ function QuerySourceCtrl( }; $scope.addNewParameter = () => { - EditParameterSettingsDialog - .showModal({ - parameter: { - title: null, - name: '', - type: 'text', - value: null, - }, - existingParams: map($scope.query.getParameters().get(), p => p.name), - }) - .result.then((param) => { - param = $scope.query.getParameters().add(param); - $rootScope.$broadcast('query-editor.command', 'paste', param.toQueryTextFragment()); - $rootScope.$broadcast('query-editor.command', 'focus'); - }); + EditParameterSettingsDialog.showModal({ + parameter: { + title: null, + name: "", + type: "text", + value: null, + }, + existingParams: map($scope.query.getParameters().get(), (p) => p.name), + }).result.then((param) => { + param = $scope.query.getParameters().add(param); + $rootScope.$broadcast( + "query-editor.command", + "paste", + param.toQueryTextFragment() + ); + $rootScope.$broadcast("query-editor.command", "focus"); + }); }; $scope.onParametersUpdated = () => { @@ -103,44 +107,62 @@ function QuerySourceCtrl( } }; - $scope.listenForEditorCommand = f => $scope.$on('query-editor.command', f); - $scope.listenForResize = f => $scope.$parent.$on('angular-resizable.resizing', f); + $scope.listenForEditorCommand = (f) => $scope.$on("query-editor.command", f); + $scope.listenForResize = (f) => + $scope.$parent.$on("angular-resizable.resizing", f); - $scope.$watch('query.query', (newQueryText) => { + $scope.$watch("query.query", (newQueryText) => { $scope.isDirty = newQueryText !== queryText; }); + + $scope.unsavedParameters = null; + $scope.getUnsavedParameters = () => { + if (!$scope.isDirty || !queryText) { + return null; + } + const unsavedParameters = + $scope.query.$parameters.getUnsavedParameters(queryText); + if (isEmpty(unsavedParameters)) { + return null; + } + // avoiding Angular infdig (ANGULAR_REMOVE_ME) + if (!isEqual(unsavedParameters, $scope.unsavedParameters)) { + $scope.unsavedParameters = unsavedParameters; + } + return $scope.unsavedParameters; + }; } export default function init(ngModule) { - ngModule.controller('QuerySourceCtrl', QuerySourceCtrl); + ngModule.controller("QuerySourceCtrl", QuerySourceCtrl); return { - '/queries/new': { + "/queries/new": { template, - layout: 'fixed', - controller: 'QuerySourceCtrl', + layout: "fixed", + controller: "QuerySourceCtrl", reloadOnSearch: false, resolve: { query: function newQuery(Query) { - 'ngInject'; + "ngInject"; return Query.newQuery(); }, dataSources(DataSource) { - 'ngInject'; + "ngInject"; return DataSource.query().$promise; }, }, }, - '/queries/:queryId/source': { + "/queries/:queryId/source": { template, - layout: 'fixed', - controller: 'QuerySourceCtrl', + layout: "fixed", + controller: "QuerySourceCtrl", reloadOnSearch: false, resolve: { query: (Query, $route) => { - 'ngInject'; + "ngInject"; return Query.get({ id: $route.current.params.queryId }).$promise; }, diff --git a/client/app/services/dashboard.js b/client/app/services/dashboard.js index 509ab387cf..89f19d6c23 100644 --- a/client/app/services/dashboard.js +++ b/client/app/services/dashboard.js @@ -1,6 +1,7 @@ -import _ from 'lodash'; -import dashboardGridOptions from '@/config/dashboard-grid-options'; -import { Widget } from './widget'; +import dashboardGridOptions from "@/config/dashboard-grid-options"; +import _ from "lodash"; + +import { Widget } from "./widget"; export let Dashboard = null; // eslint-disable-line import/no-mutable-exports @@ -25,7 +26,10 @@ export function collectDashboardFilters(dashboard, queryResults, urlParams) { if (!_.has(filters, queryFilter.name)) { filters[filter.name] = filter; } else { - filters[filter.name].values = _.union(filters[filter.name].values, filter.values); + filters[filter.name].values = _.union( + filters[filter.name].values, + filter.values + ); } }); }); @@ -36,15 +40,15 @@ export function collectDashboardFilters(dashboard, queryResults, urlParams) { function prepareWidgetsForDashboard(widgets) { // Default height for auto-height widgets. // Compute biggest widget size and choose between it and some magic number. - // This value should be big enough so auto-height widgets will not overlap other ones. + // This value should be big enough so auto-height widgets will not overlap + // other ones. const defaultWidgetSizeY = Math.max( - _ - .chain(widgets) - .map(w => w.options.position.sizeY) + _.chain(widgets) + .map((w) => w.options.position.sizeY) .max() .value(), - 20, + 20 ) + 5; // Fix layout: @@ -52,14 +56,16 @@ function prepareWidgetsForDashboard(widgets) { // 2. update position of widgets in each row - place it right below // biggest widget from previous row _.chain(widgets) - .sortBy(widget => widget.options.position.row) - .groupBy(widget => widget.options.position.row) + .sortBy((widget) => widget.options.position.row) + .groupBy((widget) => widget.options.position.row) .reduce((row, widgetsAtRow) => { let height = 1; _.each(widgetsAtRow, (widget) => { height = Math.max( height, - widget.options.position.autoHeight ? defaultWidgetSizeY : widget.options.position.sizeY, + widget.options.position.autoHeight + ? defaultWidgetSizeY + : widget.options.position.sizeY ); widget.options.position.row = row; if (widget.options.position.sizeY < 1) { @@ -71,18 +77,20 @@ function prepareWidgetsForDashboard(widgets) { .value(); // Sort widgets by updated column and row value - widgets = _.sortBy(widgets, widget => widget.options.position.col); - widgets = _.sortBy(widgets, widget => widget.options.position.row); + widgets = _.sortBy(widgets, (widget) => widget.options.position.col); + widgets = _.sortBy(widgets, (widget) => widget.options.position.row); return widgets; } function calculateNewWidgetPosition(existingWidgets, newWidget) { - const width = _.extend({ sizeX: dashboardGridOptions.defaultSizeX }, _.extend({}, newWidget.options).position).sizeX; + const width = _.extend( + { sizeX: dashboardGridOptions.defaultSizeX }, + _.extend({}, newWidget.options).position + ).sizeX; // Find first free row for each column - const bottomLine = _ - .chain(existingWidgets) + const bottomLine = _.chain(existingWidgets) .map((w) => { const options = _.extend({}, w.options); const position = _.extend({ row: 0, sizeY: 0 }, options.position); @@ -108,24 +116,24 @@ function calculateNewWidgetPosition(existingWidgets, newWidget) { // Go through columns, pick them by count necessary to hold new block, // and calculate bottom-most free row per group. // Choose group with the top-most free row (comparing to other groups) - return _ - .chain(_.range(0, dashboardGridOptions.columns - width + 1)) - .map(col => ({ + return _.chain(_.range(0, dashboardGridOptions.columns - width + 1)) + .map((col) => ({ col, - row: _ - .chain(bottomLine) + row: _.chain(bottomLine) .slice(col, col + width) .max() .value(), })) - .sortBy('row') + .sortBy("row") .first() .value(); } function DashboardService($resource, $http, $location, currentUser) { function prepareDashboardWidgets(widgets) { - return prepareWidgetsForDashboard(_.map(widgets, widget => new Widget(widget))); + return prepareWidgetsForDashboard( + _.map(widgets, (widget) => new Widget(widget)) + ); } function transformSingle(dashboard) { @@ -145,36 +153,36 @@ function DashboardService($resource, $http, $location, currentUser) { }); const resource = $resource( - 'api/dashboards/:slug', - { slug: '@slug' }, + "api/dashboards/:slug", + { slug: "@slug" }, { - get: { method: 'GET', transformResponse: transform }, - save: { method: 'POST', transformResponse: transform }, - query: { method: 'GET', isArray: false, transformResponse: transform }, + get: { method: "GET", transformResponse: transform }, + save: { method: "POST", transformResponse: transform }, + query: { method: "GET", isArray: false, transformResponse: transform }, recent: { - method: 'get', + method: "get", isArray: true, - url: 'api/dashboards/recent', + url: "api/dashboards/recent", transformResponse: transform, }, favorites: { - method: 'get', + method: "get", isArray: false, - url: 'api/dashboards/favorites', + url: "api/dashboards/favorites", }, favorite: { - method: 'post', + method: "post", isArray: false, - url: 'api/dashboards/:slug/favorite', - transformRequest: [() => ''], // body not needed + url: "api/dashboards/:slug/favorite", + transformRequest: [() => ""], // body not needed }, unfavorite: { - method: 'delete', + method: "delete", isArray: false, - url: 'api/dashboards/:slug/favorite', - transformRequest: [() => ''], // body not needed + url: "api/dashboards/:slug/favorite", + transformRequest: [() => ""], // body not needed }, - }, + } ); resource.prototype.canEdit = function canEdit() { @@ -199,7 +207,8 @@ function DashboardService($resource, $http, $location, currentUser) { if (!globalParams[mapping.mapTo]) { globalParams[mapping.mapTo] = param.clone(); globalParams[mapping.mapTo].name = mapping.mapTo; - globalParams[mapping.mapTo].title = mapping.title || param.title; + globalParams[mapping.mapTo].title = + mapping.title || param.title; globalParams[mapping.mapTo].locals = []; } @@ -209,13 +218,18 @@ function DashboardService($resource, $http, $location, currentUser) { }); } }); - return _.values(_.each(globalParams, (param) => { - param.setValue(param.value); // apply global param value to all locals - param.fromUrlParams(queryParams); // try to initialize from url (may do nothing) - })); + return _.values( + _.each(globalParams, (param) => { + param.setValue(param.value); // apply global param value to all locals + param.fromUrlParams(queryParams); // try to initialize from url (may do nothing) + }) + ); }; - resource.prototype.addWidget = function addWidget(textOrVisualization, options = {}) { + resource.prototype.addWidget = function addWidget( + textOrVisualization, + options = {} + ) { const props = { dashboard_id: this.id, options: { @@ -223,7 +237,7 @@ function DashboardService($resource, $http, $location, currentUser) { isHidden: false, position: {}, }, - text: '', + text: "", visualization_id: null, visualization: null, }; @@ -249,14 +263,51 @@ function DashboardService($resource, $http, $location, currentUser) { }); }; + let currentQueryResultsErrorData; // swap for useMemo ANGULAR_REMOVE_ME + resource.prototype.getQueryResultsErrorData = + function getQueryResultsErrorData() { + const dashboardErrors = _.map(this.widgets, (widget) => { + // get result + const result = widget.getQueryResult(); + if (!result) { + return null; + } + + // get error data + const errorData = result.getErrorData(); + if (_.isEmpty(errorData)) { + return null; + } + + // dashboard params only + const localParamNames = _.map( + widget.getLocalParameters(), + (p) => p.name + ); + const filtered = _.omit(errorData.parameters, localParamNames); + + return filtered; + }); + + const merged = _.assign({}, ...dashboardErrors); + const errorData = _.isEmpty(merged) ? null : { parameters: merged }; + + // avoiding Angular infdig (ANGULAR_REMOVE_ME) + if (!_.isEqual(currentQueryResultsErrorData, errorData)) { + currentQueryResultsErrorData = errorData; + } + + return currentQueryResultsErrorData; + }; + return resource; } export default function init(ngModule) { - ngModule.factory('Dashboard', DashboardService); + ngModule.factory("Dashboard", DashboardService); ngModule.run(($injector) => { - Dashboard = $injector.get('Dashboard'); + Dashboard = $injector.get("Dashboard"); }); } diff --git a/client/app/services/parameters/NumberParameter.js b/client/app/services/parameters/NumberParameter.js index 997cdb4b29..9c78b7caab 100644 --- a/client/app/services/parameters/NumberParameter.js +++ b/client/app/services/parameters/NumberParameter.js @@ -1,5 +1,6 @@ -import { toNumber, isNull } from 'lodash'; -import { Parameter } from '.'; +import { toNumber, trim } from "lodash"; + +import { Parameter } from "."; class NumberParameter extends Parameter { constructor(parameter, parentQueryId) { @@ -9,11 +10,11 @@ class NumberParameter extends Parameter { // eslint-disable-next-line class-methods-use-this normalizeValue(value) { - if (isNull(value)) { + if (!trim(value)) { return null; } const normalizedValue = toNumber(value); - return !isNaN(normalizedValue) ? normalizedValue : null; + return !isNaN(normalizedValue) ? normalizedValue : value; } } diff --git a/client/app/services/parameters/TextParameter.js b/client/app/services/parameters/TextParameter.js index 645adbc8e3..d92a338c68 100644 --- a/client/app/services/parameters/TextParameter.js +++ b/client/app/services/parameters/TextParameter.js @@ -1,5 +1,6 @@ -import { toString, isEmpty } from 'lodash'; -import { Parameter } from '.'; +import { isEmpty, toString, trim } from "lodash"; + +import { Parameter } from "."; class TextParameter extends Parameter { constructor(parameter, parentQueryId) { @@ -15,6 +16,13 @@ class TextParameter extends Parameter { } return normalizedValue; } + + getExecutionValue() { + if (!trim(this.value)) { + return null; + } + return this.value; + } } export default TextParameter; diff --git a/client/app/services/parameters/tests/NumberParameter.test.js b/client/app/services/parameters/tests/NumberParameter.test.js index 2a292ea880..12c3556bfe 100644 --- a/client/app/services/parameters/tests/NumberParameter.test.js +++ b/client/app/services/parameters/tests/NumberParameter.test.js @@ -1,26 +1,21 @@ -import { Parameter } from '..'; +import { Parameter } from ".."; -describe('NumberParameter', () => { +describe("NumberParameter", () => { let param; beforeEach(() => { - param = Parameter.create({ name: 'param', title: 'Param', type: 'number' }); + param = Parameter.create({ name: "param", title: "Param", type: "number" }); }); - describe('normalizeValue', () => { - test('converts Strings', () => { - const normalizedValue = param.normalizeValue('15'); + describe("normalizeValue", () => { + test("converts Strings", () => { + const normalizedValue = param.normalizeValue("15"); expect(normalizedValue).toBe(15); }); - test('converts Numbers', () => { + test("converts Numbers", () => { const normalizedValue = param.normalizeValue(42); expect(normalizedValue).toBe(42); }); - - test('returns null when not possible to convert to number', () => { - const normalizedValue = param.normalizeValue('notanumber'); - expect(normalizedValue).toBeNull(); - }); }); }); diff --git a/client/app/services/query-result.js b/client/app/services/query-result.js index a267cee050..f6cf8f3ac6 100644 --- a/client/app/services/query-result.js +++ b/client/app/services/query-result.js @@ -1,22 +1,30 @@ -import debug from 'debug'; -import moment from 'moment'; -import { uniqBy, each, isNumber, isString, includes, extend, forOwn } from 'lodash'; - -const logger = debug('redash:services:QueryResult'); -const filterTypes = ['filter', 'multi-filter', 'multiFilter']; +import debug from "debug"; +import { + each, + extend, + forOwn, + includes, + isNumber, + isString, + uniqBy, +} from "lodash"; +import moment from "moment"; + +const logger = debug("redash:services:QueryResult"); +const filterTypes = ["filter", "multi-filter", "multiFilter"]; function getColumnNameWithoutType(column) { let typeSplit; - if (column.indexOf('::') !== -1) { - typeSplit = '::'; - } else if (column.indexOf('__') !== -1) { - typeSplit = '__'; + if (column.indexOf("::") !== -1) { + typeSplit = "::"; + } else if (column.indexOf("__") !== -1) { + typeSplit = "__"; } else { return column; } const parts = column.split(typeSplit); - if (parts[0] === '' && parts.length === 2) { + if (parts[0] === "" && parts.length === 2) { return parts[1]; } @@ -32,31 +40,45 @@ export function getColumnCleanName(column) { } function getColumnFriendlyName(column) { - return getColumnNameWithoutType(column).replace(/(?:^|\s)\S/g, a => a.toUpperCase()); + return getColumnNameWithoutType(column).replace(/(?:^|\s)\S/g, (a) => + a.toUpperCase() + ); } function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { - const QueryResultResource = $resource('api/query_results/:id', { id: '@id' }, { post: { method: 'POST' } }); - const QueryResultByQueryIdResource = $resource('api/queries/:queryId/results/:id.json', { queryId: '@queryId', id: '@id' }); - const Job = $resource('api/jobs/:id', { id: '@id' }); - const JobWithApiKey = $resource('api/queries/:queryId/jobs/:id', { queryId: '@queryId', id: '@id' }); + const QueryResultResource = $resource( + "api/query_results/:id", + { id: "@id" }, + { post: { method: "POST" } } + ); + const QueryResultByQueryIdResource = $resource( + "api/queries/:queryId/results/:id.json", + { queryId: "@queryId", id: "@id" } + ); + const Job = $resource("api/jobs/:id", { id: "@id" }); + const JobWithApiKey = $resource("api/queries/:queryId/jobs/:id", { + queryId: "@queryId", + id: "@id", + }); const statuses = { - 1: 'waiting', - 2: 'processing', - 3: 'done', - 4: 'failed', + 1: "waiting", + 2: "processing", + 3: "done", + 4: "failed", }; function handleErrorResponse(queryResult, response) { if (response.status === 403) { queryResult.update(response.data); - } else if (response.status === 400 && 'job' in response.data) { + } else if (response.status === 400 && "job" in response.data) { queryResult.update(response.data); } else { - logger('Unknown error', response); + logger("Unknown error", response); queryResult.update({ job: { - error: response.data.message || 'unknown error occurred. Please try again later.', + error: + response.data.message || + "unknown error occurred. Please try again later.", status: 4, }, }); @@ -68,7 +90,7 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { this.deferred = $q.defer(); this.job = {}; this.query_result = {}; - this.status = 'waiting'; + this.status = "waiting"; this.updatedAt = moment(); @@ -83,34 +105,35 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { update(props) { extend(this, props); - if ('query_result' in props) { - this.status = 'done'; + if ("query_result" in props) { + this.status = "done"; const columnTypes = {}; - // TODO: we should stop manipulating incoming data, and switch to relaying - // on the column type set by the backend. This logic is prone to errors, - // and better be removed. Kept for now, for backward compatability. + // TODO: we should stop manipulating incoming data, and switch to + // relaying on the column type set by the backend. This logic is prone + // to errors, and better be removed. Kept for now, for backward + // compatability. each(this.query_result.data.rows, (row) => { forOwn(row, (v, k) => { let newType = null; if (isNumber(v)) { - newType = 'float'; + newType = "float"; } else if (isString(v) && v.match(/^\d{4}-\d{2}-\d{2}T/)) { row[k] = moment.utc(v); - newType = 'datetime'; + newType = "datetime"; } else if (isString(v) && v.match(/^\d{4}-\d{2}-\d{2}$/)) { row[k] = moment.utc(v); - newType = 'date'; - } else if (typeof v === 'object' && v !== null) { + newType = "date"; + } else if (typeof v === "object" && v !== null) { row[k] = JSON.stringify(v); } else { - newType = 'string'; + newType = "string"; } if (newType !== null) { if (columnTypes[k] !== undefined && columnTypes[k] !== newType) { - columnTypes[k] = 'string'; + columnTypes[k] = "string"; } else { columnTypes[k] = newType; } @@ -119,9 +142,9 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { }); each(this.query_result.data.columns, (column) => { - column.name = '' + column.name; + column.name = "" + column.name; if (columnTypes[column.name]) { - if (column.type == null || column.type === 'string') { + if (column.type == null || column.type === "string") { column.type = columnTypes[column.name]; } } @@ -129,10 +152,12 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { this.deferred.resolve(this); } else if (this.job.status === 3) { - this.status = 'processing'; + this.status = "processing"; } else if (this.job.status === 4) { this.status = statuses[this.job.status]; - this.deferred.reject(new QueryResultError(this.job.error)); + this.deferred.reject( + new QueryResultError(this.job.error, this.job.error_data) + ); } else { this.status = undefined; } @@ -140,7 +165,7 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { getId() { let id = null; - if ('query_result' in this) { + if ("query_result" in this) { id = this.query_result.id; } return id; @@ -152,22 +177,30 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { getStatus() { if (this.isLoadingResult) { - return 'loading-result'; + return "loading-result"; } return this.status || statuses[this.job.status]; } getError() { // TODO: move this logic to the server... - if (this.job.error === 'None') { + if (this.job.error === "None") { return undefined; } return this.job.error; } + getErrorData() { + return this.job.error_data || undefined; + } + getLog() { - if (!this.query_result.data || !this.query_result.data.log || this.query_result.data.log.length === 0) { + if ( + !this.query_result.data || + !this.query_result.data.log || + this.query_result.data.log.length === 0 + ) { return null; } @@ -175,7 +208,11 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { } getUpdatedAt() { - return this.query_result.retrieved_at || this.job.updated_at * 1000.0 || this.updatedAt; + return ( + this.query_result.retrieved_at || + this.job.updated_at * 1000.0 || + this.updatedAt + ); } getRuntime() { @@ -208,18 +245,18 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { getColumnNames() { if (this.columnNames === undefined && this.query_result.data) { - this.columnNames = this.query_result.data.columns.map(v => v.name); + this.columnNames = this.query_result.data.columns.map((v) => v.name); } return this.columnNames; } getColumnCleanNames() { - return this.getColumnNames().map(col => getColumnCleanName(col)); + return this.getColumnNames().map((col) => getColumnCleanName(col)); } getColumnFriendlyNames() { - return this.getColumnNames().map(col => getColumnFriendlyName(col)); + return this.getColumnNames().map((col) => getColumnFriendlyName(col)); } getFilters() { @@ -231,7 +268,7 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { this.getColumns().forEach((col) => { const name = col.name; - const type = name.split('::')[1] || name.split('__')[1]; + const type = name.split("::")[1] || name.split("__")[1]; if (includes(filterTypes, type)) { // filter found const filter = { @@ -239,7 +276,7 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { friendlyName: getColumnFriendlyName(name), column: col, values: [], - multiple: type === 'multiFilter' || type === 'multi-filter', + multiple: type === "multiFilter" || type === "multi-filter", }; filters.push(filter); } @@ -289,17 +326,26 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { // Error handler queryResult.isLoadingResult = false; handleErrorResponse(queryResult, error); - }, + } ); return queryResult; } loadLatestCachedResult(queryId, parameters) { - $resource('api/queries/:id/results', { id: '@queryId' }, { post: { method: 'POST' } }) - .post({ queryId, parameters }, - (response) => { this.update(response); }, - (error) => { handleErrorResponse(this, error); }); + $resource( + "api/queries/:id/results", + { id: "@queryId" }, + { post: { method: "POST" } } + ).post( + { queryId, parameters }, + (response) => { + this.update(response); + }, + (error) => { + handleErrorResponse(this, error); + } + ); } loadResult(tryCount) { @@ -316,10 +362,11 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { } if (tryCount > 3) { - logger('Connection error while trying to load result', error); + logger("Connection error while trying to load result", error); this.update({ job: { - error: 'failed communicating with server. Please check your Internet connection and try again.', + error: + "failed communicating with server. Please check your Internet connection and try again.", status: 4, }, }); @@ -329,25 +376,32 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { this.loadResult(tryCount + 1); }, 1000 * Math.pow(2, tryCount)); } - }, + } ); } refreshStatus(query, parameters, tryNumber = 1) { const resource = Auth.isAuthenticated() ? Job : JobWithApiKey; - const loadResult = () => (Auth.isAuthenticated() - ? this.loadResult() - : this.loadLatestCachedResult(query, parameters)); - const params = Auth.isAuthenticated() ? { id: this.job.id } : { queryId: query, id: this.job.id }; + const loadResult = () => + Auth.isAuthenticated() + ? this.loadResult() + : this.loadLatestCachedResult(query, parameters); + const params = Auth.isAuthenticated() + ? { id: this.job.id } + : { queryId: query, id: this.job.id }; resource.get( params, (jobResponse) => { this.update(jobResponse); - if (this.getStatus() === 'processing' && this.job.query_result_id && this.job.query_result_id !== 'None') { + if ( + this.getStatus() === "processing" && + this.job.query_result_id && + this.job.query_result_id !== "None" + ) { loadResult(); - } else if (this.getStatus() !== 'failed') { + } else if (this.getStatus() !== "failed") { const waitTime = tryNumber > 10 ? 3000 : 500; $timeout(() => { this.refreshStatus(query, parameters, tryNumber + 1); @@ -355,15 +409,17 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { } }, (error) => { - logger('Connection error', error); - // TODO: use QueryResultError, or better yet: exception/reject of promise. + logger("Connection error", error); + // TODO: use QueryResultError, or better yet: exception/reject of + // promise. this.update({ job: { - error: 'failed communicating with server. Please check your Internet connection and try again.', + error: + "failed communicating with server. Please check your Internet connection and try again.", status: 4, }, }); - }, + } ); } @@ -376,13 +432,20 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { } getName(queryName, fileType) { - return `${queryName.replace(/ /g, '_') + moment(this.getUpdatedAt()).format('_YYYY_MM_DD')}.${fileType}`; + return `${ + queryName.replace(/ /g, "_") + + moment(this.getUpdatedAt()).format("_YYYY_MM_DD") + }.${fileType}`; } static getByQueryId(id, parameters, maxAge) { const queryResult = new QueryResult(); - $resource('api/queries/:id/results', { id: '@id' }, { post: { method: 'POST' } }).post( + $resource( + "api/queries/:id/results", + { id: "@id" }, + { post: { method: "POST" } } + ).post( { id, parameters, @@ -391,13 +454,13 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { (response) => { queryResult.update(response); - if ('job' in response) { + if ("job" in response) { queryResult.refreshStatus(id, parameters); } }, (error) => { handleErrorResponse(queryResult, error); - }, + } ); return queryResult; @@ -422,13 +485,13 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { (response) => { queryResult.update(response); - if ('job' in response) { + if ("job" in response) { queryResult.refreshStatus(query, parameters); } }, (error) => { handleErrorResponse(queryResult, error); - }, + } ); return queryResult; @@ -439,7 +502,7 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) { } export default function init(ngModule) { - ngModule.factory('QueryResult', QueryResultService); + ngModule.factory("QueryResult", QueryResultService); } init.init = true; diff --git a/client/app/services/query.js b/client/app/services/query.js index 4c85c550df..7f72698ffd 100644 --- a/client/app/services/query.js +++ b/client/app/services/query.js @@ -1,26 +1,37 @@ -import moment from 'moment'; -import debug from 'debug'; -import Mustache from 'mustache'; +import debug from "debug"; import { - zipObject, isEmpty, map, filter, includes, union, - uniq, has, identity, extend, each, some, -} from 'lodash'; - -import { Parameter } from './parameters'; + each, + extend, + has, + identity, + includes, + isEmpty, + isNil, + map, + reject, + some, + union, + uniq, + zipObject, +} from "lodash"; +import moment from "moment"; +import Mustache from "mustache"; + +import { Parameter } from "./parameters"; Mustache.escape = identity; // do not html-escape values export let Query = null; // eslint-disable-line import/no-mutable-exports -const logger = debug('redash:services:query'); +const logger = debug("redash:services:query"); function collectParams(parts) { let parameters = []; parts.forEach((part) => { - if (part[0] === 'name' || part[0] === '&') { - parameters.push(part[1].split('.')[0]); - } else if (part[0] === '#') { + if (part[0] === "name" || part[0] === "&") { + parameters.push(part[1].split(".")[0]); + } else if (part[0] === "#") { parameters = union(parameters, collectParams(part[4])); } }); @@ -35,16 +46,16 @@ class Parameters { this.initFromQueryString(queryString); } - parseQuery() { - const fallback = () => map(this.query.options.parameters, i => i.name); + parseQuery(queryText = this.query.query) { + const fallback = () => map(this.query.options.parameters, (i) => i.name); let parameters = []; - if (this.query.query !== undefined) { + if (!isNil(queryText)) { try { - const parts = Mustache.parse(this.query.query); + const parts = Mustache.parse(queryText); parameters = uniq(collectParams(parts)); } catch (e) { - logger('Failed parsing parameters: ', e); + logger("Failed parsing parameters: ", e); // Return current parameters so we don't reset the list parameters = fallback(); } @@ -61,7 +72,9 @@ class Parameters { } this.cachedQueryText = this.query.query; - const parameterNames = update ? this.parseQuery() : map(this.query.options.parameters, p => p.name); + const parameterNames = update + ? this.parseQuery() + : map(this.query.options.parameters, (p) => p.name); this.query.options.parameters = this.query.options.parameters || []; @@ -72,20 +85,25 @@ class Parameters { parameterNames.forEach((param) => { if (!has(parametersMap, param)) { - this.query.options.parameters.push(Parameter.create({ - title: param, - name: param, - type: 'text', - value: null, - global: false, - })); + this.query.options.parameters.push( + Parameter.create({ + title: param, + name: param, + type: "text", + value: null, + global: false, + }) + ); } }); - const parameterExists = p => includes(parameterNames, p.name); + const parameterExists = (p) => includes(parameterNames, p.name); const parameters = this.query.options.parameters; - this.query.options.parameters = parameters.filter(parameterExists) - .map(p => (p instanceof Parameter ? p : Parameter.create(p, this.query.id))); + this.query.options.parameters = parameters + .filter(parameterExists) + .map((p) => + p instanceof Parameter ? p : Parameter.create(p, this.query.id) + ); } initFromQueryString(query) { @@ -100,52 +118,61 @@ class Parameters { } add(parameterDef) { - this.query.options.parameters = this.query.options.parameters - .filter(p => p.name !== parameterDef.name); + this.query.options.parameters = this.query.options.parameters.filter( + (p) => p.name !== parameterDef.name + ); const param = Parameter.create(parameterDef); this.query.options.parameters.push(param); return param; } - getMissing() { - return map(filter(this.get(), p => p.isEmpty), i => i.title); - } - isRequired() { return !isEmpty(this.get()); } getExecutionValues(extra = {}) { const params = this.get(); - return zipObject(map(params, i => i.name), map(params, i => i.getExecutionValue(extra))); + return zipObject( + map(params, (i) => i.name), + map(params, (i) => i.getExecutionValue(extra)) + ); } hasPendingValues() { - return some(this.get(), p => p.hasPendingValue); + return some(this.get(), (p) => p.hasPendingValue); } applyPendingValues() { - each(this.get(), p => p.applyPendingValue()); + each(this.get(), (p) => p.applyPendingValue()); + } + + getUnsavedParameters(queryText) { + const savedParameters = this.parseQuery(queryText); + return reject(this.get(), (p) => includes(savedParameters, p.name)).map( + (p) => p.name + ); } toUrlParams() { if (this.get().length === 0) { - return ''; + return ""; } - const params = Object.assign(...this.get().map(p => p.toUrlParams())); - Object.keys(params).forEach(key => params[key] == null && delete params[key]); - return Object - .keys(params) - .map(k => `${encodeURIComponent(k)}=${encodeURIComponent(params[k])}`) - .join('&'); + const params = Object.assign(...this.get().map((p) => p.toUrlParams())); + Object.keys(params).forEach( + (key) => params[key] == null && delete params[key] + ); + return Object.keys(params) + .map((k) => `${encodeURIComponent(k)}=${encodeURIComponent(params[k])}`) + .join("&"); } } function QueryResultErrorFactory($q) { class QueryResultError { - constructor(errorMessage) { + constructor(errorMessage, errorData = {}) { this.errorMessage = errorMessage; + this.errorData = errorData; this.updatedAt = moment.utc(); } @@ -157,13 +184,17 @@ function QueryResultErrorFactory($q) { return this.errorMessage; } + getErrorData() { + return this.errorData || undefined; + } + toPromise() { return $q.reject(this); } // eslint-disable-next-line class-methods-use-this getStatus() { - return 'failed'; + return "failed"; } // eslint-disable-next-line class-methods-use-this @@ -187,75 +218,75 @@ function QueryResource( $q, currentUser, QueryResultError, - QueryResult, + QueryResult ) { const QueryService = $resource( - 'api/queries/:id', - { id: '@id' }, + "api/queries/:id", + { id: "@id" }, { recent: { - method: 'get', + method: "get", isArray: true, - url: 'api/queries/recent', + url: "api/queries/recent", }, archive: { - method: 'get', + method: "get", isArray: false, - url: 'api/queries/archive', + url: "api/queries/archive", }, query: { isArray: false, }, myQueries: { - method: 'get', + method: "get", isArray: false, - url: 'api/queries/my', + url: "api/queries/my", }, fork: { - method: 'post', + method: "post", isArray: false, - url: 'api/queries/:id/fork', - params: { id: '@id' }, + url: "api/queries/:id/fork", + params: { id: "@id" }, }, resultById: { - method: 'get', + method: "get", isArray: false, - url: 'api/queries/:id/results.json', + url: "api/queries/:id/results.json", }, asDropdown: { - method: 'get', + method: "get", isArray: true, - url: 'api/queries/:id/dropdown', + url: "api/queries/:id/dropdown", }, associatedDropdown: { - method: 'get', + method: "get", isArray: true, - url: 'api/queries/:queryId/dropdowns/:dropdownQueryId', + url: "api/queries/:queryId/dropdowns/:dropdownQueryId", }, favorites: { - method: 'get', + method: "get", isArray: false, - url: 'api/queries/favorites', + url: "api/queries/favorites", }, favorite: { - method: 'post', + method: "post", isArray: false, - url: 'api/queries/:id/favorite', - transformRequest: [() => ''], // body not needed + url: "api/queries/:id/favorite", + transformRequest: [() => ""], // body not needed }, unfavorite: { - method: 'delete', + method: "delete", isArray: false, - url: 'api/queries/:id/favorite', - transformRequest: [() => ''], // body not needed + url: "api/queries/:id/favorite", + transformRequest: [() => ""], // body not needed }, - }, + } ); QueryService.newQuery = function newQuery() { return new QueryService({ - query: '', - name: 'New Query', + query: "", + name: "New Query", schedule: null, user: currentUser, options: {}, @@ -263,17 +294,21 @@ function QueryResource( }; QueryService.format = function formatQuery(syntax, query) { - if (syntax === 'json') { + if (syntax === "json") { try { - const formatted = JSON.stringify(JSON.parse(query), ' ', 4); + const formatted = JSON.stringify(JSON.parse(query), " ", 4); return $q.resolve(formatted); } catch (err) { return $q.reject(String(err)); } - } else if (syntax === 'sql') { - return $http.post('api/queries/format', { query }).then(response => response.data.query); + } else if (syntax === "sql") { + return $http + .post("api/queries/format", { query }) + .then((response) => response.data.query); } else { - return $q.reject('Query formatting is not supported for your data source syntax.'); + return $q.reject( + "Query formatting is not supported for your data source syntax." + ); } }; @@ -290,13 +325,8 @@ function QueryResource( }; QueryService.prototype.scheduleInLocalTime = function scheduleInLocalTime() { - const parts = this.schedule.split(':'); - return moment - .utc() - .hour(parts[0]) - .minute(parts[1]) - .local() - .format('HH:mm'); + const parts = this.schedule.split(":"); + return moment.utc().hour(parts[0]).minute(parts[1]).local().format("HH:mm"); }; QueryService.prototype.hasResult = function hasResult() { @@ -311,62 +341,67 @@ function QueryResource( return this.getParametersDefs().length > 0; }; - QueryService.prototype.prepareQueryResultExecution = function prepareQueryResultExecution(execute, maxAge) { - const parameters = this.getParameters(); - const missingParams = parameters.getMissing(); + QueryService.prototype.prepareQueryResultExecution = + function prepareQueryResultExecution(execute, maxAge) { + const parameters = this.getParameters(); - if (missingParams.length > 0) { - let paramsWord = 'parameter'; - let valuesWord = 'value'; - if (missingParams.length > 1) { - paramsWord = 'parameters'; - valuesWord = 'values'; + if (parameters.isRequired()) { + // Need to clear latest results, to make sure we don't use results for + // different params. + this.latest_query_data = null; + this.latest_query_data_id = null; } - return new QueryResult({ - job: { - error: `missing ${valuesWord} for ${missingParams.join(', ')} ${paramsWord}.`, - status: 4, - }, - }); - } - - if (parameters.isRequired()) { - // Need to clear latest results, to make sure we don't use results for different params. - this.latest_query_data = null; - this.latest_query_data_id = null; - } - - if (this.latest_query_data && maxAge !== 0) { - if (!this.queryResult) { - this.queryResult = new QueryResult({ - query_result: this.latest_query_data, - }); + if (this.latest_query_data && maxAge !== 0) { + if (!this.queryResult) { + this.queryResult = new QueryResult({ + query_result: this.latest_query_data, + }); + } + } else if (this.latest_query_data_id && maxAge !== 0) { + if (!this.queryResult) { + this.queryResult = QueryResult.getById( + this.id, + this.latest_query_data_id + ); + } + } else { + this.queryResult = execute(); } - } else if (this.latest_query_data_id && maxAge !== 0) { - if (!this.queryResult) { - this.queryResult = QueryResult.getById(this.id, this.latest_query_data_id); - } - } else { - this.queryResult = execute(); - } - return this.queryResult; - }; + return this.queryResult; + }; QueryService.prototype.getQueryResult = function getQueryResult(maxAge) { - const execute = () => QueryResult.getByQueryId(this.id, this.getParameters().getExecutionValues(), maxAge); + const execute = () => + QueryResult.getByQueryId( + this.id, + this.getParameters().getExecutionValues(), + maxAge + ); return this.prepareQueryResultExecution(execute, maxAge); }; - QueryService.prototype.getQueryResultByText = function getQueryResultByText(maxAge, selectedQueryText) { + QueryService.prototype.getQueryResultByText = function getQueryResultByText( + maxAge, + selectedQueryText + ) { const queryText = selectedQueryText || this.query; if (!queryText) { return new QueryResultError("Can't execute empty query."); } - const parameters = this.getParameters().getExecutionValues({ joinListValues: true }); - const execute = () => QueryResult.get(this.data_source_id, queryText, parameters, maxAge, this.id); + const parameters = this.getParameters().getExecutionValues({ + joinListValues: true, + }); + const execute = () => + QueryResult.get( + this.data_source_id, + queryText, + parameters, + maxAge, + this.id + ); return this.prepareQueryResultExecution(execute, maxAge); }; @@ -374,7 +409,7 @@ function QueryResource( let url = `queries/${this.id}`; if (source) { - url += '/source'; + url += "/source"; } let params = {}; @@ -383,10 +418,16 @@ function QueryResource( extend(params, param.toUrlParams()); }); } - Object.keys(params).forEach(key => params[key] == null && delete params[key]); - params = map(params, (value, name) => `${encodeURIComponent(name)}=${encodeURIComponent(value)}`).join('&'); - - if (params !== '') { + Object.keys(params).forEach( + (key) => params[key] == null && delete params[key] + ); + params = map( + params, + (value, name) => + `${encodeURIComponent(name)}=${encodeURIComponent(value)}` + ).join("&"); + + if (params !== "") { url += `?${params}`; } @@ -397,9 +438,10 @@ function QueryResource( return url; }; - QueryService.prototype.getQueryResultPromise = function getQueryResultPromise() { - return this.getQueryResult().toPromise(); - }; + QueryService.prototype.getQueryResultPromise = + function getQueryResultPromise() { + return this.getQueryResult().toPromise(); + }; QueryService.prototype.getParameters = function getParameters() { if (!this.$parameters) { @@ -409,7 +451,9 @@ function QueryResource( return this.$parameters; }; - QueryService.prototype.getParametersDefs = function getParametersDefs(update = true) { + QueryService.prototype.getParametersDefs = function getParametersDefs( + update = true + ) { return this.getParameters().get(update); }; @@ -417,11 +461,11 @@ function QueryResource( } export default function init(ngModule) { - ngModule.factory('QueryResultError', QueryResultErrorFactory); - ngModule.factory('Query', QueryResource); + ngModule.factory("QueryResultError", QueryResultErrorFactory); + ngModule.factory("Query", QueryResource); ngModule.run(($injector) => { - Query = $injector.get('Query'); + Query = $injector.get("Query"); }); } diff --git a/client/cypress/integration/query/parameter_spec.js b/client/cypress/integration/query/parameter_spec.js index 35fb9afd7c..2753e11df6 100644 --- a/client/cypress/integration/query/parameter_spec.js +++ b/client/cypress/integration/query/parameter_spec.js @@ -1,145 +1,185 @@ -import { createQuery } from '../../support/redash-api'; +import { + addWidget, + createDashboard, + createQuery, +} from "../../support/redash-api"; -describe('Parameter', () => { +const { get } = Cypress._; + +describe("Parameter", () => { const expectDirtyStateChange = (edit) => { - cy.getByTestId('ParameterName-test-parameter') - .find('.parameter-input') + cy.getByTestId("ParameterName-test-parameter") + .find(".parameter-input") .should(($el) => { - assert.isUndefined($el.data('dirty')); + assert.isUndefined($el.data("dirty")); }); edit(); - cy.getByTestId('ParameterName-test-parameter') - .find('.parameter-input') + cy.getByTestId("ParameterName-test-parameter") + .find(".parameter-input") .should(($el) => { - assert.isTrue($el.data('dirty')); + assert.isTrue($el.data("dirty")); }); }; + const expectValueValidationError = ( + edit, + expectedInvalidString = "Required parameter" + ) => { + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-form-item-control") + .should("have.class", "has-error") + .find(".ant-form-explain") + .should("contain.text", expectedInvalidString) + .should("not.have.class", "show-help-enter"); // assures ant animation ended for screenshot + }; + beforeEach(() => { cy.login(); }); - describe('Text Parameter', () => { + describe("Text Parameter", () => { beforeEach(() => { const queryData = { - name: 'Text Parameter', + name: "Text Parameter", query: "SELECT '{{test-parameter}}' AS parameter", options: { parameters: [ - { name: 'test-parameter', title: 'Test Parameter', type: 'text' }, + { + name: "test-parameter", + title: "Test Parameter", + type: "text", + value: "text", + }, ], }, }; - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}`) + ); }); - it('updates the results after clicking Apply', () => { - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .type('Redash'); + it("updates the results after clicking Apply", () => { + cy.getByTestId("ParameterName-test-parameter") + .find("input") + .type("Redash"); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', 'Redash'); + cy.getByTestId("TableVisualization").should("contain", "Redash"); }); - it('sets dirty state when edited', () => { + it("sets dirty state when edited", () => { expectDirtyStateChange(() => { - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .type('Redash'); + cy.getByTestId("ParameterName-test-parameter") + .find("input") + .type("Redash"); }); }); + + it("shows validation error when value is empty", () => { + cy.getByTestId("ParameterName-test-parameter").find("input").clear(); + + cy.getByTestId("ParameterApplyButton").click(); + + expectValueValidationError(); + }); }); - describe('Number Parameter', () => { + describe("Number Parameter", () => { beforeEach(() => { const queryData = { - name: 'Number Parameter', + name: "Number Parameter", query: "SELECT '{{test-parameter}}' AS parameter", options: { parameters: [ - { name: 'test-parameter', title: 'Test Parameter', type: 'number' }, + { + name: "test-parameter", + title: "Test Parameter", + type: "number", + value: 1, + }, ], }, }; - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}`) + ); }); - it('updates the results after clicking Apply', () => { - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .type('{selectall}42'); + it("updates the results after clicking Apply", () => { + cy.getByTestId("ParameterName-test-parameter") + .find("input") + .type("{selectall}42"); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', 42); + cy.getByTestId("TableVisualization").should("contain", 42); - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .type('{selectall}31415'); + cy.getByTestId("ParameterName-test-parameter") + .find("input") + .type("{selectall}31415"); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', 31415); + cy.getByTestId("TableVisualization").should("contain", 31415); }); - it('sets dirty state when edited', () => { + it("sets dirty state when edited", () => { expectDirtyStateChange(() => { - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .type('{selectall}42'); + cy.getByTestId("ParameterName-test-parameter") + .find("input") + .type("{selectall}42"); }); }); + + it("shows validation error when value is empty", () => { + cy.getByTestId("ParameterName-test-parameter").find("input").clear(); + + cy.getByTestId("ParameterApplyButton").click(); + + expectValueValidationError(); + }); }); - describe('Dropdown Parameter', () => { + describe("Dropdown Parameter", () => { beforeEach(() => { const queryData = { - name: 'Dropdown Parameter', + name: "Dropdown Parameter", query: "SELECT '{{test-parameter}}' AS parameter", options: { parameters: [ - { name: 'test-parameter', - title: 'Test Parameter', - type: 'enum', - enumOptions: 'value1\nvalue2\nvalue3' }, + { + name: "test-parameter", + title: "Test Parameter", + type: "enum", + enumOptions: "value1\nvalue2\nvalue3", + }, ], }, }; - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}/source`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}/source`) + ); }); - it('updates the results after selecting a value', () => { - cy.getByTestId('ParameterName-test-parameter') - .find('.ant-select') + it("updates the results after selecting a value", () => { + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-select") .click(); - cy.contains('li.ant-select-dropdown-menu-item', 'value2') - .click(); + cy.contains("li.ant-select-dropdown-menu-item", "value2").click(); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', 'value2'); + cy.getByTestId("TableVisualization").should("contain", "value2"); }); - it('supports multi-selection', () => { + it("supports multi-selection", () => { cy.clickThrough(` ParameterSettings-test-parameter AllowMultipleValuesCheckbox @@ -148,73 +188,105 @@ describe('Parameter', () => { SaveParameterSettings `); - cy.getByTestId('ParameterName-test-parameter') - .find('.ant-select') + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-select") .click(); // select all unselected options - cy.get('li.ant-select-dropdown-menu-item').each(($option) => { - if (!$option.hasClass('ant-select-dropdown-menu-item-selected')) { + cy.get("li.ant-select-dropdown-menu-item").each(($option) => { + if (!$option.hasClass("ant-select-dropdown-menu-item-selected")) { cy.wrap($option).click(); } }); - cy.getByTestId('QueryEditor').click(); // just to close the select menu + cy.getByTestId("QueryEditor").click(); // just to close the select menu - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', '"value1","value2","value3"'); + cy.getByTestId("TableVisualization").should( + "contain", + '"value1","value2","value3"' + ); }); - it('sets dirty state when edited', () => { + it("sets dirty state when edited", () => { expectDirtyStateChange(() => { - cy.getByTestId('ParameterName-test-parameter') - .find('.ant-select') + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-select") .click(); - cy.contains('li.ant-select-dropdown-menu-item', 'value2') - .click(); + cy.contains("li.ant-select-dropdown-menu-item", "value2").click(); }); }); + + it("shows validation error when empty", () => { + cy.getByTestId("ParameterSettings-test-parameter").click(); + cy.getByTestId("EnumTextArea").clear(); + cy.clickThrough(` + SaveParameterSettings + ExecuteButton + `); + + expectValueValidationError(); + }); + + it("shows validation error when multi-selection is empty", () => { + cy.clickThrough(` + ParameterSettings-test-parameter + AllowMultipleValuesCheckbox + QuotationSelect + DoubleQuotationMarkOption + SaveParameterSettings + `); + + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-select-remove-icon") + .click(); + + cy.getByTestId("ParameterApplyButton").click(); + + expectValueValidationError(); + }); }); - describe('Query Based Dropdown Parameter', () => { + describe("Query Based Dropdown Parameter", () => { beforeEach(() => { const dropdownQueryData = { - name: 'Dropdown Query', + name: "Dropdown Query", query: `SELECT 'value1' AS name, 1 AS value UNION ALL SELECT 'value2' AS name, 2 AS value UNION ALL SELECT 'value3' AS name, 3 AS value`, }; createQuery(dropdownQueryData, true).then((dropdownQuery) => { const queryData = { - name: 'Query Based Dropdown Parameter', + name: "Query Based Dropdown Parameter", query: "SELECT '{{test-parameter}}' AS parameter", options: { parameters: [ - { name: 'test-parameter', - title: 'Test Parameter', - type: 'query', - queryId: dropdownQuery.id }, + { + name: "test-parameter", + title: "Test Parameter", + type: "query", + queryId: dropdownQuery.id, + }, ], }, }; cy.visit(`/queries/${dropdownQuery.id}`); - cy.getByTestId('ExecuteButton').click(); - cy.getByTestId('TableVisualization') - .should('contain', 'value1') - .and('contain', 'value2') - .and('contain', 'value3'); - - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}/source`)); + cy.getByTestId("ExecuteButton").click(); + cy.getByTestId("TableVisualization") + .should("contain", "value1") + .and("contain", "value2") + .and("contain", "value3"); + + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}/source`) + ); }); }); - it('supports multi-selection', () => { + it("supports multi-selection", () => { cy.clickThrough(` ParameterSettings-test-parameter AllowMultipleValuesCheckbox @@ -223,428 +295,611 @@ describe('Parameter', () => { SaveParameterSettings `); - cy.getByTestId('ParameterName-test-parameter') - .find('.ant-select') + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-select") .click(); // make sure all options are unselected and select all - cy.get('li.ant-select-dropdown-menu-item').each(($option) => { - expect($option).not.to.have.class('ant-select-dropdown-menu-item-selected'); + cy.get("li.ant-select-dropdown-menu-item").each(($option) => { + expect($option).not.to.have.class( + "ant-select-dropdown-menu-item-selected" + ); cy.wrap($option).click(); }); - cy.getByTestId('QueryEditor').click(); // just to close the select menu + cy.getByTestId("QueryEditor").click(); // just to close the select menu - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', '"1","2","3"'); + cy.getByTestId("TableVisualization").should("contain", '"1","2","3"'); }); }); - describe('Date Parameter', () => { + describe("Date Parameter", () => { const selectCalendarDate = (date) => { - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .click({ force: true }); + cy.getByTestId("ParameterName-test-parameter").find("input").click({ + force: true, + }); - cy.get('.ant-calendar-date-panel') - .contains('.ant-calendar-date', date) + cy.get(".ant-calendar-date-panel") + .contains(".ant-calendar-date", date) .click(); }; beforeEach(() => { const queryData = { - name: 'Date Parameter', + name: "Date Parameter", query: "SELECT '{{test-parameter}}' AS parameter", options: { parameters: [ - { name: 'test-parameter', title: 'Test Parameter', type: 'date', value: null }, + { + name: "test-parameter", + title: "Test Parameter", + type: "date", + value: null, + }, ], }, }; const now = new Date(); now.setDate(1); - cy.wrap(now.getTime()).as('now'); - cy.clock(now.getTime(), ['Date']); + cy.wrap(now.getTime()).as("now"); + cy.clock(now.getTime(), ["Date"]); - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}`) + ); }); afterEach(() => { - cy.clock().then(clock => clock.restore()); + cy.clock().then((clock) => clock.restore()); }); - it('updates the results after selecting a date', function () { - selectCalendarDate('15'); + it("updates the results after selecting a date", function () { + selectCalendarDate("15"); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', Cypress.moment(this.now).format('15/MM/YY')); + cy.getByTestId("TableVisualization").should( + "contain", + Cypress.moment(this.now).format("15/MM/YY") + ); }); - it('allows picking a dynamic date', function () { - cy.getByTestId('DynamicButton') - .click(); + it("allows picking a dynamic date", function () { + cy.getByTestId("DynamicButton").click(); - cy.getByTestId('DynamicButtonMenu') - .contains('Today/Now') - .click(); + cy.getByTestId("DynamicButtonMenu").contains("Today/Now").click(); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); + + cy.getByTestId("TableVisualization").should( + "contain", + Cypress.moment(this.now).format("DD/MM/YY") + ); + }); - cy.getByTestId('TableVisualization') - .should('contain', Cypress.moment(this.now).format('DD/MM/YY')); + it("sets dirty state when edited", () => { + expectDirtyStateChange(() => selectCalendarDate("15")); }); - it('sets dirty state when edited', () => { - expectDirtyStateChange(() => selectCalendarDate('15')); + it("shows validation error when value is empty", () => { + selectCalendarDate("15"); + + cy.getByTestId("ParameterApplyButton").click(); + + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-calendar-picker-clear") + .click({ force: true }); + + cy.getByTestId("ParameterApplyButton").click(); + + expectValueValidationError(); }); }); - describe('Date and Time Parameter', () => { + describe("Date and Time Parameter", () => { beforeEach(() => { const queryData = { - name: 'Date and Time Parameter', + name: "Date and Time Parameter", query: "SELECT '{{test-parameter}}' AS parameter", options: { parameters: [ - { name: 'test-parameter', title: 'Test Parameter', type: 'datetime-local', value: null }, + { + name: "test-parameter", + title: "Test Parameter", + type: "datetime-local", + value: null, + }, ], }, }; const now = new Date(); now.setDate(1); - cy.wrap(now.getTime()).as('now'); - cy.clock(now.getTime(), ['Date']); + cy.wrap(now.getTime()).as("now"); + cy.clock(now.getTime(), ["Date"]); - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}`) + ); }); afterEach(() => { - cy.clock().then(clock => clock.restore()); + cy.clock().then((clock) => clock.restore()); }); - it('updates the results after selecting a date and clicking in ok', function () { - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .as('Input') + it("updates the results after selecting a date and clicking in ok", function () { + cy.getByTestId("ParameterName-test-parameter") + .find("input") + .as("Input") .click({ force: true }); - cy.get('.ant-calendar-date-panel') - .contains('.ant-calendar-date', '15') + cy.get(".ant-calendar-date-panel") + .contains(".ant-calendar-date", "15") .click(); - cy.get('.ant-calendar-ok-btn') - .click(); + cy.get(".ant-calendar-ok-btn").click(); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', Cypress.moment(this.now).format('YYYY-MM-15 HH:mm')); + cy.getByTestId("TableVisualization").should( + "contain", + Cypress.moment(this.now).format("YYYY-MM-15 HH:mm") + ); }); - it('shows the current datetime after clicking in Now', function () { - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .as('Input') + it("shows the current datetime after clicking in Now", function () { + cy.getByTestId("ParameterName-test-parameter") + .find("input") + .as("Input") .click({ force: true }); - cy.get('.ant-calendar-date-panel') - .contains('Now') - .click(); + cy.get(".ant-calendar-date-panel").contains("Now").click(); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', Cypress.moment(this.now).format('YYYY-MM-DD HH:mm')); + cy.getByTestId("TableVisualization").should( + "contain", + Cypress.moment(this.now).format("YYYY-MM-DD HH:mm") + ); }); - it('allows picking a dynamic date', function () { - cy.getByTestId('DynamicButton') - .click(); + it("allows picking a dynamic date", function () { + cy.getByTestId("DynamicButton").click(); - cy.getByTestId('DynamicButtonMenu') - .contains('Today/Now') - .click(); + cy.getByTestId("DynamicButtonMenu").contains("Today/Now").click(); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - cy.getByTestId('TableVisualization') - .should('contain', Cypress.moment(this.now).format('YYYY-MM-DD HH:mm')); + cy.getByTestId("TableVisualization").should( + "contain", + Cypress.moment(this.now).format("YYYY-MM-DD HH:mm") + ); }); - it('sets dirty state when edited', () => { + it("sets dirty state when edited", () => { expectDirtyStateChange(() => { - cy.getByTestId('ParameterName-test-parameter') - .find('input') - .click({ force: true }); + cy.getByTestId("ParameterName-test-parameter").find("input").click({ + force: true, + }); - cy.get('.ant-calendar-date-panel') - .contains('Now') - .click(); + cy.get(".ant-calendar-date-panel").contains("Now").click(); }); }); + + it("shows validation error when value is empty", () => { + cy.getByTestId("ParameterName-test-parameter") + .find("input") + .as("Input") + .click({ force: true }); + + cy.get(".ant-calendar-date-panel") + .contains(".ant-calendar-date", "15") + .click(); + + cy.get(".ant-calendar-ok-btn").click(); + + cy.getByTestId("ParameterApplyButton").click(); + + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-calendar-picker-clear") + .click({ force: true }); + + cy.getByTestId("ParameterApplyButton").click(); + + expectValueValidationError(); + }); }); - describe('Date Range Parameter', () => { + describe("Date Range Parameter", () => { const selectCalendarDateRange = (startDate, endDate) => { - cy.getByTestId('ParameterName-test-parameter') - .find('input') + cy.getByTestId("ParameterName-test-parameter") + .find("input") .first() .click({ force: true }); - cy.get('.ant-calendar-date-panel') - .contains('.ant-calendar-date', startDate) + cy.get(".ant-calendar-date-panel") + .contains(".ant-calendar-date", startDate) .click(); - cy.get('.ant-calendar-date-panel') - .contains('.ant-calendar-date', endDate) + cy.get(".ant-calendar-date-panel") + .contains(".ant-calendar-date", endDate) .click(); }; beforeEach(() => { const queryData = { - name: 'Date Range Parameter', - query: "SELECT '{{test-parameter.start}} - {{test-parameter.end}}' AS parameter", + name: "Date Range Parameter", + query: + "SELECT '{{test-parameter.start}} - {{test-parameter.end}}' AS parameter", options: { parameters: [ - { name: 'test-parameter', title: 'Test Parameter', type: 'date-range' }, + { + name: "test-parameter", + title: "Test Parameter", + type: "date-range", + }, ], }, }; const now = new Date(); now.setDate(1); - cy.wrap(now.getTime()).as('now'); - cy.clock(now.getTime(), ['Date']); + cy.wrap(now.getTime()).as("now"); + cy.clock(now.getTime(), ["Date"]); - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}/source`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}/source`) + ); }); afterEach(() => { - cy.clock().then(clock => clock.restore()); + cy.clock().then((clock) => clock.restore()); }); - it('updates the results after selecting a date range', function () { - selectCalendarDateRange('15', '20'); + it("updates the results after selecting a date range", function () { + selectCalendarDateRange("15", "20"); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); const now = Cypress.moment(this.now); - cy.getByTestId('TableVisualization') - .should('contain', now.format('YYYY-MM-15') + ' - ' + now.format('YYYY-MM-20')); + cy.getByTestId("TableVisualization").should( + "contain", + now.format("YYYY-MM-15") + " - " + now.format("YYYY-MM-20") + ); }); - it('allows picking a dynamic date range', function () { - cy.getByTestId('DynamicButton') - .click(); + it("allows picking a dynamic date range", function () { + cy.getByTestId("DynamicButton").click(); - cy.getByTestId('DynamicButtonMenu') - .contains('Last month') - .click(); + cy.getByTestId("DynamicButtonMenu").contains("Last month").click(); - cy.getByTestId('ParameterApplyButton') - .click(); + cy.getByTestId("ParameterApplyButton").click(); - const lastMonth = Cypress.moment(this.now).subtract(1, 'month'); - cy.getByTestId('TableVisualization') - .should('contain', lastMonth.startOf('month').format('YYYY-MM-DD') + ' - ' + - lastMonth.endOf('month').format('YYYY-MM-DD')); + const lastMonth = Cypress.moment(this.now).subtract(1, "month"); + cy.getByTestId("TableVisualization").should( + "contain", + lastMonth.startOf("month").format("YYYY-MM-DD") + + " - " + + lastMonth.endOf("month").format("YYYY-MM-DD") + ); }); - it('sets dirty state when edited', () => { - expectDirtyStateChange(() => selectCalendarDateRange('15', '20')); + it("sets dirty state when edited", () => { + expectDirtyStateChange(() => selectCalendarDateRange("15", "20")); + }); + + it("shows validation error when value is empty", () => { + selectCalendarDateRange("15", "20"); + + cy.getByTestId("ParameterApplyButton").click(); + + cy.getByTestId("ParameterName-test-parameter") + .find(".ant-calendar-picker-clear") + .click({ force: true }); + + cy.getByTestId("ParameterApplyButton").click(); + + expectValueValidationError(); + }); + }); + + describe("Inline feedback", () => { + beforeEach(function () { + const queryData = { + query: "SELECT {{ test-parameter }}", + options: { + parameters: [ + { + name: "test-parameter", + title: "Param", + type: "number", + value: null, + }, + ], + }, + }; + + createQuery(queryData, false).then((query) => { + this.query = query; + this.vizId = get(query, "visualizations.0.id"); + }); + }); + + it("shows validation error in query page", function () { + cy.visit(`/queries/${this.query.id}`); + expectValueValidationError(); + cy.percySnapshot("Validation error in query page"); + }); + + it("shows unsaved feedback in query page", function () { + cy.visit(`/queries/${this.query.id}/source`); + + cy.getByTestId("QueryEditor") + .get(".ace_text-input") + .type(" {{ newparam }}", { + force: true, + parseSpecialCharSequences: false, + }); + + cy.getByTestId("ParameterName-newparam") + .find(".ant-form-item-control") + .should("have.class", "has-warning") + .find(".ant-form-explain") + .as("Feedback"); + + cy.get("@Feedback") + .should("contain.text", "Unsaved") + .should("not.have.class", "show-help-appear"); // assures ant animation ended for screenshot + + cy.percySnapshot("Unsaved feedback in query page"); + + cy.getByTestId("SaveButton").click(); + cy.get("@Feedback").should("not.exist"); + }); + + it("shows validation error in visualization embed", function () { + cy.visit( + `/embed/query/${this.query.id}/visualization/${this.vizId}?api_key=${this.query.api_key}` + ); + expectValueValidationError(); + cy.percySnapshot("Validation error in visualization embed"); + }); + + it("shows validation error in widget-level parameter", function () { + createDashboard("Foo") + .then(({ slug, id }) => { + this.dashboardUrl = `/dashboard/${slug}`; + return addWidget(id, this.vizId, { + parameterMappings: { + "test-parameter": { + type: "widget-level", + title: "", + name: "test-parameter", + mapTo: "test-parameter", + value: null, + }, + }, + }); + }) + .then(() => { + cy.visit(this.dashboardUrl); + }); + expectValueValidationError(); + cy.percySnapshot("Validation error in widget-level parameter"); + }); + + it("shows validation error in dashboard-level parameter", function () { + createDashboard("Foo") + .then(({ slug, id }) => { + this.dashboardUrl = `/dashboard/${slug}`; + return addWidget(id, this.vizId, { + parameterMappings: { + "test-parameter": { + type: "dashboard-level", + title: "", + name: "test-parameter", + mapTo: "test-parameter", + value: null, + }, + }, + }); + }) + .then(() => { + cy.visit(this.dashboardUrl); + }); + expectValueValidationError(); + cy.percySnapshot("Validation error in dashboard-level parameter"); }); }); - describe('Apply Changes', () => { + describe("Apply Changes", () => { const expectAppliedChanges = (apply) => { - cy.getByTestId('ParameterName-test-parameter-1') - .find('input') - .as('Input') - .type('Redash'); + cy.getByTestId("ParameterName-test-parameter-1") + .find("input") + .as("Input") + .type("Redash"); - cy.getByTestId('ParameterName-test-parameter-2') - .find('input') - .type('Redash'); + cy.getByTestId("ParameterName-test-parameter-2") + .find("input") + .type("Redash"); - cy.location('search').should('not.contain', 'Redash'); + cy.location("search").should("not.contain", "Redash"); cy.server(); - cy.route('POST', 'api/queries/*/results').as('Results'); + cy.route("POST", "api/queries/*/results").as("Results"); - apply(cy.get('@Input')); + apply(cy.get("@Input")); - cy.location('search').should('contain', 'Redash'); - cy.wait('@Results'); + cy.location("search").should("contain", "Redash"); + cy.wait("@Results"); }; beforeEach(() => { const queryData = { - name: 'Testing Apply Button', + name: "Testing Apply Button", query: "SELECT '{{test-parameter-1}} {{ test-parameter-2 }}'", options: { parameters: [ - { name: 'test-parameter-1', title: 'Test Parameter 1', type: 'text' }, - { name: 'test-parameter-2', title: 'Test Parameter 2', type: 'text' }, + { + name: "test-parameter-1", + title: "Test Parameter 1", + type: "text", + }, + { + name: "test-parameter-2", + title: "Test Parameter 2", + type: "text", + }, ], }, }; - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}/source`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}/source`) + ); }); - it('shows and hides according to parameter dirty state', () => { - cy.getByTestId('ParameterApplyButton') - .should('not.be', 'visible'); + it("shows and hides according to parameter dirty state", () => { + cy.getByTestId("ParameterApplyButton").should("not.be", "visible"); - cy.getByTestId('ParameterName-test-parameter-1') - .find('input') - .as('Param') - .type('Redash'); + cy.getByTestId("ParameterName-test-parameter-1") + .find("input") + .as("Param") + .type("Redash"); - cy.getByTestId('ParameterApplyButton') - .should('be', 'visible'); + cy.getByTestId("ParameterApplyButton").should("be", "visible"); - cy.get('@Param') - .clear(); + cy.get("@Param").clear(); - cy.getByTestId('ParameterApplyButton') - .should('not.be', 'visible'); + cy.getByTestId("ParameterApplyButton").should("not.be", "visible"); }); - it('updates dirty counter', () => { - cy.getByTestId('ParameterName-test-parameter-1') - .find('input') - .type('Redash'); + it("updates dirty counter", () => { + cy.getByTestId("ParameterName-test-parameter-1") + .find("input") + .type("Redash"); - cy.getByTestId('ParameterApplyButton') - .find('.ant-badge-count p.current') - .should('contain', '1'); + cy.getByTestId("ParameterApplyButton") + .find(".ant-badge-count p.current") + .should("contain", "1"); - cy.getByTestId('ParameterName-test-parameter-2') - .find('input') - .type('Redash'); + cy.getByTestId("ParameterName-test-parameter-2") + .find("input") + .type("Redash"); - cy.getByTestId('ParameterApplyButton') - .find('.ant-badge-count p.current') - .should('contain', '2'); + cy.getByTestId("ParameterApplyButton") + .find(".ant-badge-count p.current") + .should("contain", "2"); }); it('applies changes from "Apply Changes" button', () => { expectAppliedChanges(() => { - cy.getByTestId('ParameterApplyButton').click(); + cy.getByTestId("ParameterApplyButton").click(); }); }); it('applies changes from "alt+enter" keyboard shortcut', () => { expectAppliedChanges((input) => { - input.type('{alt}{enter}'); + input.type("{alt}{enter}"); }); }); it('disables "Execute" button', () => { - cy.getByTestId('ParameterName-test-parameter-1') - .find('input') - .as('Input') - .type('Redash'); - cy.getByTestId('ExecuteButton').should('be.disabled'); + cy.getByTestId("ParameterName-test-parameter-1") + .find("input") + .as("Input") + .type("Redash"); + cy.getByTestId("ExecuteButton").should("be.disabled"); - cy.get('@Input').clear(); - cy.getByTestId('ExecuteButton').should('not.be.disabled'); + cy.get("@Input").clear(); + cy.getByTestId("ExecuteButton").should("not.be.disabled"); }); }); - describe('Draggable', () => { + describe("Draggable", () => { beforeEach(() => { const queryData = { - name: 'Draggable', - query: "SELECT '{{param1}}', '{{param2}}', '{{param3}}', '{{param4}}' AS parameter", + name: "Draggable", + query: + "SELECT '{{param1}}', '{{param2}}', '{{param3}}', '{{param4}}' AS parameter", options: { parameters: [ - { name: 'param1', title: 'Parameter 1', type: 'text' }, - { name: 'param2', title: 'Parameter 2', type: 'text' }, - { name: 'param3', title: 'Parameter 3', type: 'text' }, - { name: 'param4', title: 'Parameter 4', type: 'text' }, + { name: "param1", title: "Parameter 1", type: "text" }, + { name: "param2", title: "Parameter 2", type: "text" }, + { name: "param3", title: "Parameter 3", type: "text" }, + { name: "param4", title: "Parameter 4", type: "text" }, ], }, }; - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}/source`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}/source`) + ); - cy.get('.parameter-block') - .first() - .invoke('width') - .as('paramWidth'); + cy.get(".parameter-block").first().invoke("width").as("paramWidth"); }); const dragParam = (paramName, offsetLeft, offsetTop) => { cy.getByTestId(`DragHandle-${paramName}`) - .trigger('mouseover') - .trigger('mousedown'); + .trigger("mouseover") + .trigger("mousedown"); - cy.get('.parameter-dragged .drag-handle') - .trigger('mousemove', offsetLeft, offsetTop, { force: true }) - .trigger('mouseup', { force: true }); + cy.get(".parameter-dragged .drag-handle") + .trigger("mousemove", offsetLeft, offsetTop, { force: true }) + .trigger("mouseup", { force: true }); }; - it('is possible to rearrange parameters', function () { - dragParam('param1', this.paramWidth, 1); - dragParam('param4', -this.paramWidth, 1); + it("is possible to rearrange parameters", function () { + dragParam("param1", this.paramWidth, 1); + dragParam("param4", -this.paramWidth, 1); cy.reload(); - const expectedOrder = ['Parameter 2', 'Parameter 1', 'Parameter 4', 'Parameter 3']; - cy.get('.parameter-container label') - .each(($label, index) => expect($label).to.have.text(expectedOrder[index])); + const expectedOrder = [ + "Parameter 2", + "Parameter 1", + "Parameter 4", + "Parameter 3", + ]; + cy.get(".parameter-container label").each(($label, index) => + expect($label).to.have.text(expectedOrder[index]) + ); }); }); - describe('Parameter Settings', () => { + describe("Parameter Settings", () => { beforeEach(() => { const queryData = { - name: 'Draggable', + name: "Draggable", query: "SELECT '{{parameter}}' AS parameter", options: { - parameters: [ - { name: 'parameter', title: 'Parameter', type: 'text' }, - ], + parameters: [{ name: "parameter", title: "Parameter", type: "text" }], }, }; - createQuery(queryData, false) - .then(({ id }) => cy.visit(`/queries/${id}/source`)); + createQuery(queryData, false).then(({ id }) => + cy.visit(`/queries/${id}/source`) + ); - cy.getByTestId('ParameterSettings-parameter').click(); + cy.getByTestId("ParameterSettings-parameter").click(); }); - it('changes the parameter title', () => { - cy.getByTestId('ParameterTitleInput') - .type('{selectall}New Parameter Name'); - cy.getByTestId('SaveParameterSettings') - .click(); + it("changes the parameter title", () => { + cy.getByTestId("ParameterTitleInput").type( + "{selectall}New Parameter Name" + ); + cy.getByTestId("SaveParameterSettings").click(); - cy.contains('Query saved'); + cy.contains("Query saved"); cy.reload(); - cy.getByTestId('ParameterName-parameter') - .contains('label', 'New Parameter Name'); + cy.getByTestId("ParameterName-parameter").contains( + "label", + "New Parameter Name" + ); }); }); }); diff --git a/redash/handlers/query_results.py b/redash/handlers/query_results.py index 28d458863e..3b8b2a2d07 100644 --- a/redash/handlers/query_results.py +++ b/redash/handlers/query_results.py @@ -1,86 +1,146 @@ -import logging import time -from flask import make_response, request +from flask import make_response +from flask import request from flask_login import current_user from flask_restful import abort -from redash import models, settings -from redash.handlers.base import BaseResource, get_object_or_404, record_event -from redash.permissions import (has_access, not_view_only, require_access, - require_permission, view_only) + +from redash import models +from redash import settings +from redash.handlers.base import BaseResource +from redash.handlers.base import get_object_or_404 +from redash.handlers.base import record_event +from redash.models.parameterized_query import dropdown_values +from redash.models.parameterized_query import InvalidParameterError +from redash.models.parameterized_query import ParameterizedQuery +from redash.models.parameterized_query import QueryDetachedFromDataSourceError +from redash.permissions import has_access +from redash.permissions import not_view_only +from redash.permissions import require_access +from redash.permissions import require_permission +from redash.permissions import view_only +from redash.serializers import serialize_query_result +from redash.serializers import serialize_query_result_to_csv +from redash.serializers import serialize_query_result_to_xlsx from redash.tasks import QueryTask from redash.tasks.queries import enqueue_query -from redash.utils import (collect_parameters_from_request, gen_query_hash, json_dumps, utcnow, to_filename) -from redash.models.parameterized_query import (ParameterizedQuery, InvalidParameterError, - QueryDetachedFromDataSourceError, dropdown_values) -from redash.serializers import serialize_query_result, serialize_query_result_to_csv, serialize_query_result_to_xlsx +from redash.utils import collect_parameters_from_request +from redash.utils import gen_query_hash +from redash.utils import json_dumps +from redash.utils import to_filename +from redash.utils import utcnow -def error_response(message, http_status=400): - return {'job': {'status': 4, 'error': message}}, http_status +def error_response(message, data=None, http_status=400): + return { + "job": { + "status": 4, + "error": message, + "error_data": data + } + }, http_status error_messages = { - 'unsafe_when_shared': error_response('This query contains potentially unsafe parameters and cannot be executed on a shared dashboard or an embedded visualization.', 403), - 'unsafe_on_view_only': error_response('This query contains potentially unsafe parameters and cannot be executed with read-only access to this data source.', 403), - 'no_permission': error_response('You do not have permission to run queries with this data source.', 403), - 'select_data_source': error_response('Please select data source to run this query.', 401) + "unsafe_when_shared": + error_response( + "This query contains potentially unsafe parameters and cannot be executed on a shared dashboard or an embedded visualization.", + None, + 403, + ), + "unsafe_on_view_only": + error_response( + "This query contains potentially unsafe parameters and cannot be executed with read-only access to this data source.", + None, + 403, + ), + "no_permission": + error_response( + "You do not have permission to run queries with this data source.", + None, 403), + "select_data_source": + error_response("Please select data source to run this query.", None, 401), } def run_query(query, parameters, data_source, query_id, max_age=0): if data_source.paused: if data_source.pause_reason: - message = '{} is paused ({}). Please try later.'.format(data_source.name, data_source.pause_reason) + message = "{} is paused ({}). Please try later.".format( + data_source.name, data_source.pause_reason) else: - message = '{} is paused. Please try later.'.format(data_source.name) + message = "{} is paused. Please try later.".format( + data_source.name) return error_response(message) try: query.apply(parameters) - except (InvalidParameterError, QueryDetachedFromDataSourceError) as e: + except QueryDetachedFromDataSourceError as e: abort(400, message=e.message) + except InvalidParameterError as e: + return error_response(e.message, {"parameters": e.parameter_errors}) - if query.missing_params: - return error_response('Missing parameter value for: {}'.format(", ".join(query.missing_params))) + missing_params_error = query.missing_params_error + if missing_params_error: + message, parameter_errors = missing_params_error + return error_response(message, {"parameters": parameter_errors}) if max_age == 0: query_result = None else: - query_result = models.QueryResult.get_latest(data_source, query.text, max_age) - - record_event(current_user.org, current_user, { - 'action': 'execute_query', - 'cache': 'hit' if query_result else 'miss', - 'object_id': data_source.id, - 'object_type': 'data_source', - 'query': query.text, - 'query_id': query_id, - 'parameters': parameters - }) + query_result = models.QueryResult.get_latest(data_source, query.text, + max_age) + + record_event( + current_user.org, + current_user, + { + "action": "execute_query", + "cache": "hit" if query_result else "miss", + "object_id": data_source.id, + "object_type": "data_source", + "query": query.text, + "query_id": query_id, + "parameters": parameters, + }, + ) if query_result: - return {'query_result': serialize_query_result(query_result, current_user.is_api_user())} + return { + "query_result": + serialize_query_result(query_result, current_user.is_api_user()) + } else: - job = enqueue_query(query.text, data_source, current_user.id, current_user.is_api_user(), metadata={ - "Username": repr(current_user) if current_user.is_api_user() else current_user.email, - "Query ID": query_id - }) - return {'job': job.to_dict()} + job = enqueue_query( + query.text, + data_source, + current_user.id, + current_user.is_api_user(), + metadata={ + "Username": + repr(current_user) + if current_user.is_api_user() else current_user.email, + "Query ID": + query_id, + }, + ) + return {"job": job.to_dict()} def get_download_filename(query_result, query, filetype): retrieved_at = query_result.retrieved_at.strftime("%Y_%m_%d") if query: - filename = to_filename(query.name) if query.name != '' else str(query.id) + filename = to_filename(query.name) if query.name != "" else str( + query.id) else: filename = str(query_result.id) return "{}_{}.{}".format(filename, retrieved_at, filetype) class QueryResultListResource(BaseResource): - @require_permission('execute_query') + + @require_permission("execute_query") def post(self): """ Execute a query (or retrieve recent results). @@ -96,35 +156,40 @@ def post(self): """ params = request.get_json(force=True) - query = params['query'] - max_age = params.get('max_age', -1) + query = params["query"] + max_age = params.get("max_age", -1) # max_age might have the value of None, in which case calling int(None) will fail if max_age is None: max_age = -1 max_age = int(max_age) - query_id = params.get('query_id', 'adhoc') - parameters = params.get('parameters', collect_parameters_from_request(request.args)) + query_id = params.get("query_id", "adhoc") + parameters = params.get("parameters", + collect_parameters_from_request(request.args)) parameterized_query = ParameterizedQuery(query, org=self.current_org) - data_source_id = params.get('data_source_id') + data_source_id = params.get("data_source_id") if data_source_id: - data_source = models.DataSource.get_by_id_and_org(params.get('data_source_id'), self.current_org) + data_source = models.DataSource.get_by_id_and_org( + params.get("data_source_id"), self.current_org) else: - return error_messages['select_data_source'] + return error_messages["select_data_source"] if not has_access(data_source, self.current_user, not_view_only): - return error_messages['no_permission'] + return error_messages["no_permission"] - return run_query(parameterized_query, parameters, data_source, query_id, max_age) + return run_query(parameterized_query, parameters, data_source, + query_id, max_age) ONE_YEAR = 60 * 60 * 24 * 365.25 class QueryResultDropdownResource(BaseResource): + def get(self, query_id): - query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org) + query = get_object_or_404(models.Query.get_by_id_and_org, query_id, + self.current_org) require_access(query.data_source, current_user, view_only) try: return dropdown_values(query_id, self.current_org) @@ -133,42 +198,52 @@ def get(self, query_id): class QueryDropdownsResource(BaseResource): + def get(self, query_id, dropdown_query_id): - query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org) + query = get_object_or_404(models.Query.get_by_id_and_org, query_id, + self.current_org) require_access(query, current_user, view_only) - related_queries_ids = [p['queryId'] for p in query.parameters if p['type'] == 'query'] + related_queries_ids = [ + p["queryId"] for p in query.parameters if p["type"] == "query" + ] if int(dropdown_query_id) not in related_queries_ids: - dropdown_query = get_object_or_404(models.Query.get_by_id_and_org, dropdown_query_id, self.current_org) + dropdown_query = get_object_or_404(models.Query.get_by_id_and_org, + dropdown_query_id, + self.current_org) require_access(dropdown_query.data_source, current_user, view_only) return dropdown_values(dropdown_query_id, self.current_org) class QueryResultResource(BaseResource): + @staticmethod def add_cors_headers(headers): - if 'Origin' in request.headers: - origin = request.headers['Origin'] + if "Origin" in request.headers: + origin = request.headers["Origin"] - if set(['*', origin]) & settings.ACCESS_CONTROL_ALLOW_ORIGIN: - headers['Access-Control-Allow-Origin'] = origin - headers['Access-Control-Allow-Credentials'] = str(settings.ACCESS_CONTROL_ALLOW_CREDENTIALS).lower() + if set(["*", origin]) & settings.ACCESS_CONTROL_ALLOW_ORIGIN: + headers["Access-Control-Allow-Origin"] = origin + headers["Access-Control-Allow-Credentials"] = str( + settings.ACCESS_CONTROL_ALLOW_CREDENTIALS).lower() - @require_permission('view_query') - def options(self, query_id=None, query_result_id=None, filetype='json'): + @require_permission("view_query") + def options(self, query_id=None, query_result_id=None, filetype="json"): headers = {} self.add_cors_headers(headers) if settings.ACCESS_CONTROL_REQUEST_METHOD: - headers['Access-Control-Request-Method'] = settings.ACCESS_CONTROL_REQUEST_METHOD + headers[ + "Access-Control-Request-Method"] = settings.ACCESS_CONTROL_REQUEST_METHOD if settings.ACCESS_CONTROL_ALLOW_HEADERS: - headers['Access-Control-Allow-Headers'] = settings.ACCESS_CONTROL_ALLOW_HEADERS + headers[ + "Access-Control-Allow-Headers"] = settings.ACCESS_CONTROL_ALLOW_HEADERS return make_response("", 200, headers) - @require_permission('view_query') + @require_permission("view_query") def post(self, query_id): """ Execute a saved query. @@ -181,31 +256,39 @@ def post(self, query_id): always execute. """ params = request.get_json(force=True, silent=True) or {} - parameter_values = params.get('parameters', {}) + parameter_values = params.get("parameters", {}) - max_age = params.get('max_age', -1) + max_age = params.get("max_age", -1) # max_age might have the value of None, in which case calling int(None) will fail if max_age is None: max_age = -1 max_age = int(max_age) - query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org) + query = get_object_or_404(models.Query.get_by_id_and_org, query_id, + self.current_org) allow_executing_with_view_only_permissions = query.parameterized.is_safe - if has_access(query, self.current_user, allow_executing_with_view_only_permissions): - return run_query(query.parameterized, parameter_values, query.data_source, query_id, max_age) + if has_access(query, self.current_user, + allow_executing_with_view_only_permissions): + return run_query( + query.parameterized, + parameter_values, + query.data_source, + query_id, + max_age, + ) else: if not query.parameterized.is_safe: if current_user.is_api_user(): - return error_messages['unsafe_when_shared'] + return error_messages["unsafe_when_shared"] else: - return error_messages['unsafe_on_view_only'] + return error_messages["unsafe_on_view_only"] else: - return error_messages['no_permission'] + return error_messages["no_permission"] - @require_permission('view_query') - def get(self, query_id=None, query_result_id=None, filetype='json'): + @require_permission("view_query") + def get(self, query_id=None, query_result_id=None, filetype="json"): """ Retrieve query results. @@ -228,52 +311,61 @@ def get(self, query_id=None, query_result_id=None, filetype='json'): should_cache = query_result_id is not None parameter_values = collect_parameters_from_request(request.args) - max_age = int(request.args.get('maxAge', 0)) + max_age = int(request.args.get("maxAge", 0)) query_result = None query = None if query_result_id: - query_result = get_object_or_404(models.QueryResult.get_by_id_and_org, query_result_id, self.current_org) + query_result = get_object_or_404( + models.QueryResult.get_by_id_and_org, query_result_id, + self.current_org) if query_id is not None: - query = get_object_or_404(models.Query.get_by_id_and_org, query_id, self.current_org) - - if query_result is None and query is not None and query.latest_query_data_id is not None: - query_result = get_object_or_404(models.QueryResult.get_by_id_and_org, - query.latest_query_data_id, - self.current_org) - - if query is not None and query_result is not None and self.current_user.is_api_user(): + query = get_object_or_404(models.Query.get_by_id_and_org, query_id, + self.current_org) + + if (query_result is None and query is not None + and query.latest_query_data_id is not None): + query_result = get_object_or_404( + models.QueryResult.get_by_id_and_org, + query.latest_query_data_id, + self.current_org, + ) + + if (query is not None and query_result is not None + and self.current_user.is_api_user()): if query.query_hash != query_result.query_hash: - abort(404, message='No cached result found for this query.') + abort(404, + message="No cached result found for this query.") if query_result: - require_access(query_result.data_source, self.current_user, view_only) + require_access(query_result.data_source, self.current_user, + view_only) if isinstance(self.current_user, models.ApiUser): event = { - 'user_id': None, - 'org_id': self.current_org.id, - 'action': 'api_get', - 'api_key': self.current_user.name, - 'file_type': filetype, - 'user_agent': request.user_agent.string, - 'ip': request.remote_addr + "user_id": None, + "org_id": self.current_org.id, + "action": "api_get", + "api_key": self.current_user.name, + "file_type": filetype, + "user_agent": request.user_agent.string, + "ip": request.remote_addr, } if query_id: - event['object_type'] = 'query' - event['object_id'] = query_id + event["object_type"] = "query" + event["object_id"] = query_id else: - event['object_type'] = 'query_result' - event['object_id'] = query_result_id + event["object_type"] = "query_result" + event["object_id"] = query_result_id self.record_event(event) - if filetype == 'json': + if filetype == "json": response = self.make_json_response(query_result) - elif filetype == 'xlsx': + elif filetype == "xlsx": response = self.make_excel_response(query_result) else: response = self.make_csv_response(query_result) @@ -282,43 +374,49 @@ def get(self, query_id=None, query_result_id=None, filetype='json'): self.add_cors_headers(response.headers) if should_cache: - response.headers.add_header('Cache-Control', 'private,max-age=%d' % ONE_YEAR) + response.headers.add_header("Cache-Control", + "private,max-age=%d" % ONE_YEAR) filename = get_download_filename(query_result, query, filetype) response.headers.add_header( "Content-Disposition", - 'attachment; filename="{}"'.format(filename) - ) + 'attachment; filename="{}"'.format(filename)) return response else: - abort(404, message='No cached result found for this query.') + abort(404, message="No cached result found for this query.") def make_json_response(self, query_result): - data = json_dumps({'query_result': query_result.to_dict()}) - headers = {'Content-Type': "application/json"} + data = json_dumps({"query_result": query_result.to_dict()}) + headers = {"Content-Type": "application/json"} return make_response(data, 200, headers) @staticmethod def make_csv_response(query_result): - headers = {'Content-Type': "text/csv; charset=UTF-8"} - return make_response(serialize_query_result_to_csv(query_result), 200, headers) + headers = {"Content-Type": "text/csv; charset=UTF-8"} + return make_response(serialize_query_result_to_csv(query_result), 200, + headers) @staticmethod def make_excel_response(query_result): - headers = {'Content-Type': "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"} - return make_response(serialize_query_result_to_xlsx(query_result), 200, headers) + headers = { + "Content-Type": + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" + } + return make_response(serialize_query_result_to_xlsx(query_result), 200, + headers) class JobResource(BaseResource): + def get(self, job_id, query_id=None): """ Retrieve info about a running query job. """ job = QueryTask(job_id=job_id) - return {'job': job.to_dict()} + return {"job": job.to_dict()} def delete(self, job_id): """ diff --git a/redash/models/parameterized_query.py b/redash/models/parameterized_query.py index 81ddde18c6..edab30736d 100644 --- a/redash/models/parameterized_query.py +++ b/redash/models/parameterized_query.py @@ -1,12 +1,18 @@ -import pystache from functools import partial from numbers import Number -from redash.utils import mustache_render, json_loads -from redash.permissions import require_access, view_only -from funcy import distinct + +import pystache from dateutil.parser import parse +from funcy import compact +from funcy import distinct +from funcy import lpluck +from six import string_types +from six import text_type -from six import string_types, text_type +from redash.permissions import require_access +from redash.permissions import view_only +from redash.utils import json_loads +from redash.utils import mustache_render def _pluck_name_and_value(default_column, row): @@ -23,7 +29,8 @@ def _load_result(query_id, org): query = models.Query.get_by_id_and_org(query_id, org) if query.data_source: - query_result = models.QueryResult.get_by_id_and_org(query.latest_query_data_id, org) + query_result = models.QueryResult.get_by_id_and_org( + query.latest_query_data_id, org) return query_result.data else: raise QueryDetachedFromDataSourceError(query_id) @@ -38,14 +45,17 @@ def dropdown_values(query_id, org): def join_parameter_list_values(parameters, schema): updated_parameters = {} - for (key, value) in parameters.items(): + for key, value in parameters.items(): if isinstance(value, list): - definition = next((definition for definition in schema if definition["name"] == key), {}) - multi_values_options = definition.get('multiValuesOptions', {}) - separator = str(multi_values_options.get('separator', ',')) - prefix = str(multi_values_options.get('prefix', '')) - suffix = str(multi_values_options.get('suffix', '')) - updated_parameters[key] = separator.join([prefix + v + suffix for v in value]) + definition = next( + (definition + for definition in schema if definition["name"] == key), {}) + multi_values_options = definition.get("multiValuesOptions", {}) + separator = str(multi_values_options.get("separator", ",")) + prefix = str(multi_values_options.get("prefix", "")) + suffix = str(multi_values_options.get("suffix", "")) + updated_parameters[key] = separator.join( + [prefix + v + suffix for v in value]) else: updated_parameters[key] = value return updated_parameters @@ -74,7 +84,7 @@ def _parameter_names(parameter_values): for key, value in parameter_values.items(): if isinstance(value, dict): for inner_key in value.keys(): - names.append('{}.{}'.format(key, inner_key)) + names.append("{}.{}".format(key, inner_key)) else: names.append(key) @@ -107,13 +117,26 @@ def _is_date_range(obj): return False +def _is_date_range_type(type): + return type in [ + "date-range", "datetime-range", "datetime-range-with-seconds" + ] + + +def _is_tag_in_template(name, template): + tags = _collect_query_parameters(template) + return name in tags + + def _is_value_within_options(value, dropdown_options, allow_list=False): if isinstance(value, list): - return allow_list and set(map(text_type, value)).issubset(set(dropdown_options)) + return allow_list and set(map(text_type, value)).issubset( + set(dropdown_options)) return text_type(value) in dropdown_options class ParameterizedQuery(object): + def __init__(self, template, schema=None, org=None): self.schema = schema or [] self.org = org @@ -122,76 +145,164 @@ def __init__(self, template, schema=None, org=None): self.parameters = {} def apply(self, parameters): - invalid_parameter_names = [key for (key, value) in parameters.items() if not self._valid(key, value)] - if invalid_parameter_names: - raise InvalidParameterError(invalid_parameter_names) + # filter out params not defined in schema + if self.schema: + names_with_definition = lpluck("name", self.schema) + parameters = { + k: v + for (k, v) in parameters.items() if k in names_with_definition + } + + invalid_parameters = compact( + {k: self._invalid_message(k, v) + for (k, v) in parameters.items()}) + if invalid_parameters: + raise InvalidParameterError(invalid_parameters) else: self.parameters.update(parameters) - self.query = mustache_render(self.template, join_parameter_list_values(parameters, self.schema)) + self.query = mustache_render( + self.template, + join_parameter_list_values(parameters, self.schema)) return self - def _valid(self, name, value): + def _invalid_message(self, name, value): + if value is None: + return "Required parameter" + + # skip if no schema if not self.schema: - return True + return None - definition = next((definition for definition in self.schema if definition["name"] == name), None) + definition = next( + (definition + for definition in self.schema if definition["name"] == name), + None, + ) if not definition: - return False + return "Parameter no longer exists in query." - enum_options = definition.get('enumOptions') - query_id = definition.get('queryId') - allow_multiple_values = isinstance(definition.get('multiValuesOptions'), dict) + enum_options = definition.get("enumOptions") + query_id = definition.get("queryId") + allow_multiple_values = isinstance( + definition.get("multiValuesOptions"), dict) if isinstance(enum_options, string_types): - enum_options = enum_options.split('\n') - - validators = { - "text": lambda value: isinstance(value, string_types), - "number": _is_number, - "enum": lambda value: _is_value_within_options(value, - enum_options, - allow_multiple_values), - "query": lambda value: _is_value_within_options(value, - [v["value"] for v in dropdown_values(query_id, self.org)], - allow_multiple_values), - "date": _is_date, - "datetime-local": _is_date, - "datetime-with-seconds": _is_date, - "date-range": _is_date_range, - "datetime-range": _is_date_range, - "datetime-range-with-seconds": _is_date_range, + enum_options = enum_options.split("\n") + + value_validators = { + "text": + lambda value: isinstance(value, string_types), + "number": + _is_number, + "enum": + lambda value: _is_value_within_options(value, enum_options, + allow_multiple_values), + "query": + lambda value: _is_value_within_options( + value, + [v["value"] for v in dropdown_values(query_id, self.org)], + allow_multiple_values, + ), + "date": + _is_date, + "datetime-local": + _is_date, + "datetime-with-seconds": + _is_date, + "date-range": + _is_date_range, + "datetime-range": + _is_date_range, + "datetime-range-with-seconds": + _is_date_range, } - validate = validators.get(definition["type"], lambda x: False) + validate_value = value_validators.get(definition["type"], + lambda x: False) + + if not validate_value(value): + return "Invalid value" + + tag_error_msg = self._validate_tag(name, definition["type"]) + if tag_error_msg is not None: + return tag_error_msg - return validate(value) + return None + + def _validate_tag(self, name, type): + error_msg = "{{{{ {0} }}}} not found in query" + if _is_date_range_type(type): + start_tag = "{}.start".format(name) + if not _is_tag_in_template(start_tag, self.template): + return error_msg.format(start_tag) + + end_tag = "{}.end".format(name) + if not _is_tag_in_template(end_tag, self.template): + return error_msg.format(end_tag) + + elif not _is_tag_in_template(name, self.template): + return error_msg.format(name) + + return None @property def is_safe(self): - text_parameters = [param for param in self.schema if param["type"] == "text"] + text_parameters = [ + param for param in self.schema if param["type"] == "text" + ] return not any(text_parameters) @property def missing_params(self): - query_parameters = set(_collect_query_parameters(self.template)) + query_parameters = _collect_query_parameters(self.template) return set(query_parameters) - set(_parameter_names(self.parameters)) + @property + def missing_params_error(self): + missing_params = self.missing_params + if not missing_params: + return None + + parameter_names = ", ".join('"{}"'.format(name) + for name in sorted(missing_params)) + if len(missing_params) > 1: + message = "Parameters {} are missing.".format(parameter_names) + else: + message = "Parameter {} is missing.".format(parameter_names) + + parameter_errors = { + name: "Missing parameter" + for name in missing_params + } + return message, parameter_errors + @property def text(self): return self.query class InvalidParameterError(Exception): - def __init__(self, parameters): - parameter_names = ", ".join(parameters) - message = "The following parameter values are incompatible with their definitions: {}".format(parameter_names) - super(InvalidParameterError, self).__init__(message) + + def __init__(self, parameter_errors): + parameter_names = ", ".join( + '"{}"'.format(name) for name in sorted(parameter_errors.keys())) + if len(parameter_errors) > 1: + message = "Parameters {} are invalid.".format(parameter_names) + else: + message = "Parameter {} is invalid.".format(parameter_names) + + self.message = message + self.parameter_errors = parameter_errors + + super().__init__(message, parameter_errors) class QueryDetachedFromDataSourceError(Exception): + def __init__(self, query_id): self.query_id = query_id - super(QueryDetachedFromDataSourceError, self).__init__( - "This query is detached from any data source. Please select a different query.") + self.message = "This query is detached from any data source. Please select a different query." + + super().__init__(self.message) diff --git a/tests/models/test_parameterized_query.py b/tests/models/test_parameterized_query.py index dc2ac0372d..344b9b0bb7 100644 --- a/tests/models/test_parameterized_query.py +++ b/tests/models/test_parameterized_query.py @@ -1,56 +1,178 @@ -from unittest import TestCase -from mock import patch from collections import namedtuple +from unittest import TestCase + import pytest +from mock import patch -from redash.models.parameterized_query import ParameterizedQuery, InvalidParameterError, QueryDetachedFromDataSourceError, dropdown_values +from redash.models.parameterized_query import dropdown_values +from redash.models.parameterized_query import InvalidParameterError +from redash.models.parameterized_query import ParameterizedQuery +from redash.models.parameterized_query import QueryDetachedFromDataSourceError class TestParameterizedQuery(TestCase): + def test_returns_empty_list_for_regular_query(self): query = ParameterizedQuery("SELECT 1") self.assertEqual(set([]), query.missing_params) def test_finds_all_params_when_missing(self): query = ParameterizedQuery("SELECT {{param}} FROM {{table}}") - self.assertEqual(set(['param', 'table']), query.missing_params) + self.assertEqual(set(["param", "table"]), query.missing_params) def test_finds_all_params(self): query = ParameterizedQuery("SELECT {{param}} FROM {{table}}").apply({ - 'param': 'value', - 'table': 'value' + "param": + "value", + "table": + "value" }) self.assertEqual(set([]), query.missing_params) def test_deduplicates_params(self): - query = ParameterizedQuery("SELECT {{param}}, {{param}} FROM {{table}}").apply({ - 'param': 'value', - 'table': 'value' - }) + query = ParameterizedQuery( + "SELECT {{param}}, {{param}} FROM {{table}}").apply({ + "param": + "value", + "table": + "value" + }) self.assertEqual(set([]), query.missing_params) def test_handles_nested_params(self): - query = ParameterizedQuery("SELECT {{param}}, {{param}} FROM {{table}} -- {{#test}} {{nested_param}} {{/test}}").apply({ - 'param': 'value', - 'table': 'value' + query = ParameterizedQuery( + "SELECT {{param}}, {{param}} FROM {{table}} -- {{#test}} {{nested_param}} {{/test}}" + ).apply({ + "param": "value", + "table": "value" }) - self.assertEqual(set(['test', 'nested_param']), query.missing_params) + self.assertEqual(set(["test", "nested_param"]), query.missing_params) def test_handles_objects(self): - query = ParameterizedQuery("SELECT * FROM USERS WHERE created_at between '{{ created_at.start }}' and '{{ created_at.end }}'").apply({ - 'created_at': { - 'start': 1, - 'end': 2 - } - }) + query = ParameterizedQuery( + "SELECT * FROM USERS WHERE created_at between '{{ created_at.start }}' and '{{ created_at.end }}'" + ).apply({"created_at": { + "start": 1, + "end": 2 + }}) self.assertEqual(set([]), query.missing_params) - def test_raises_on_parameters_not_in_schema(self): + def test_single_invalid_parameter_exception(self): + query = ParameterizedQuery("foo") + with pytest.raises(InvalidParameterError) as excinfo: + query.apply({"bar": None}) + + message, parameter_errors = excinfo.value.args + self.assertEquals(message, 'Parameter "bar" is invalid.') + self.assertEquals(len(parameter_errors), 1) + + def test_multiple_invalid_parameter_exception(self): + query = ParameterizedQuery("foo") + with pytest.raises(InvalidParameterError) as excinfo: + query.apply({"bar": None, "baz": None}) + + message, parameter_errors = excinfo.value.args + self.assertEquals(message, 'Parameters "bar", "baz" are invalid.') + self.assertEquals(len(parameter_errors), 2) + + def test_invalid_parameter_error_messages(self): + schema = [ + { + "name": "bar", + "type": "text" + }, + { + "name": "baz", + "type": "text" + }, + { + "name": "foo", + "type": "text" + }, + { + "name": "spam", + "type": "date-range" + }, + { + "name": "ham", + "type": "date-range" + }, + { + "name": "eggs", + "type": "number" + }, + ] + parameters = { + "bar": None, + "baz": 7, + "foo": "text", + "spam": { + "start": "2000-01-01 12:00:00", + "end": "2000-12-31 12:00:00" + }, + "ham": { + "start": "2000-01-01 12:00:00", + "end": "2000-12-31 12:00:00" + }, + "eggs": 42, + } + query = ParameterizedQuery( + "foo {{ spam }} {{ ham.start}} {{ eggs.start }}", schema) + with pytest.raises(InvalidParameterError) as excinfo: + query.apply(parameters) + + _, parameter_errors = excinfo.value.args + self.assertEquals( + parameter_errors, + { + "bar": "Required parameter", + "baz": "Invalid value", + "foo": "{{ foo }} not found in query", + "spam": "{{ spam.start }} not found in query", + "ham": "{{ ham.end }} not found in query", + "eggs": "{{ eggs }} not found in query", + }, + ) + + def test_single_missing_parameter_error(self): + query = ParameterizedQuery("foo {{ bar }}") + + message, parameter_errors = query.missing_params_error + self.assertEquals(message, 'Parameter "bar" is missing.') + self.assertEquals(len(parameter_errors), 1) + + def test_multiple_missing_parameter_error(self): + query = ParameterizedQuery("foo {{ bar }} {{ baz }}") + + message, parameter_errors = query.missing_params_error + self.assertEquals(message, 'Parameters "bar", "baz" are missing.') + self.assertEquals(len(parameter_errors), 2) + + def test_missing_parameter_error_message(self): + query = ParameterizedQuery("foo {{ bar }}") + + _, parameter_errors = query.missing_params_error + self.assertEquals(parameter_errors, {"bar": "Missing parameter"}) + + def test_ignores_parameters_not_in_schema(self): + schema = [{"name": "bar", "type": "text"}] + query = ParameterizedQuery("foo {{ bar }}", schema) + + with pytest.raises(InvalidParameterError) as excinfo: + query.apply({"qux": 7, "bar": 7}) + + _, parameter_errors = excinfo.value.args + self.assertTrue("bar" in parameter_errors) + self.assertFalse("qux" in parameter_errors) + + def test_passes_on_parameters_not_in_schema(self): schema = [{"name": "bar", "type": "text"}] query = ParameterizedQuery("foo", schema) - with pytest.raises(InvalidParameterError): - query.apply({"qux": 7}) + try: + query.apply({"qux": None}) + except InvalidParameterError: + pytest.fail("Unexpected InvalidParameterError") def test_raises_on_invalid_text_parameters(self): schema = [{"name": "bar", "type": "text"}] @@ -113,14 +235,22 @@ def test_validates_date_parameters(self): self.assertEqual("foo 2000-01-01 12:00:00", query.text) def test_raises_on_invalid_enum_parameters(self): - schema = [{"name": "bar", "type": "enum", "enumOptions": ["baz", "qux"]}] + schema = [{ + "name": "bar", + "type": "enum", + "enumOptions": ["baz", "qux"] + }] query = ParameterizedQuery("foo", schema) with pytest.raises(InvalidParameterError): query.apply({"bar": 7}) def test_raises_on_unlisted_enum_value_parameters(self): - schema = [{"name": "bar", "type": "enum", "enumOptions": ["baz", "qux"]}] + schema = [{ + "name": "bar", + "type": "enum", + "enumOptions": ["baz", "qux"] + }] query = ParameterizedQuery("foo", schema) with pytest.raises(InvalidParameterError): @@ -131,7 +261,11 @@ def test_raises_on_unlisted_enum_list_value_parameters(self): "name": "bar", "type": "enum", "enumOptions": ["baz", "qux"], - "multiValuesOptions": {"separator": ",", "prefix": "", "suffix": ""} + "multiValuesOptions": { + "separator": ",", + "prefix": "", + "suffix": "" + }, }] query = ParameterizedQuery("foo", schema) @@ -139,7 +273,11 @@ def test_raises_on_unlisted_enum_list_value_parameters(self): query.apply({"bar": ["shlomo", "baz"]}) def test_validates_enum_parameters(self): - schema = [{"name": "bar", "type": "enum", "enumOptions": ["baz", "qux"]}] + schema = [{ + "name": "bar", + "type": "enum", + "enumOptions": ["baz", "qux"] + }] query = ParameterizedQuery("foo {{bar}}", schema) query.apply({"bar": "baz"}) @@ -151,7 +289,11 @@ def test_validates_enum_list_value_parameters(self): "name": "bar", "type": "enum", "enumOptions": ["baz", "qux"], - "multiValuesOptions": {"separator": ",", "prefix": "'", "suffix": "'"} + "multiValuesOptions": { + "separator": ",", + "prefix": "'", + "suffix": "'" + }, }] query = ParameterizedQuery("foo {{bar}}", schema) @@ -159,7 +301,12 @@ def test_validates_enum_list_value_parameters(self): self.assertEqual("foo 'qux','baz'", query.text) - @patch('redash.models.parameterized_query.dropdown_values', return_value=[{"value": "1"}]) + @patch( + "redash.models.parameterized_query.dropdown_values", + return_value=[{ + "value": "1" + }], + ) def test_validation_accepts_integer_values_for_dropdowns(self, _): schema = [{"name": "bar", "type": "query", "queryId": 1}] query = ParameterizedQuery("foo {{bar}}", schema) @@ -168,7 +315,7 @@ def test_validation_accepts_integer_values_for_dropdowns(self, _): self.assertEqual("foo 1", query.text) - @patch('redash.models.parameterized_query.dropdown_values') + @patch("redash.models.parameterized_query.dropdown_values") def test_raises_on_invalid_query_parameters(self, _): schema = [{"name": "bar", "type": "query", "queryId": 1}] query = ParameterizedQuery("foo", schema) @@ -176,7 +323,12 @@ def test_raises_on_invalid_query_parameters(self, _): with pytest.raises(InvalidParameterError): query.apply({"bar": 7}) - @patch('redash.models.parameterized_query.dropdown_values', return_value=[{"value": "baz"}]) + @patch( + "redash.models.parameterized_query.dropdown_values", + return_value=[{ + "value": "baz" + }], + ) def test_raises_on_unlisted_query_value_parameters(self, _): schema = [{"name": "bar", "type": "query", "queryId": 1}] query = ParameterizedQuery("foo", schema) @@ -184,7 +336,12 @@ def test_raises_on_unlisted_query_value_parameters(self, _): with pytest.raises(InvalidParameterError): query.apply({"bar": "shlomo"}) - @patch('redash.models.parameterized_query.dropdown_values', return_value=[{"value": "baz"}]) + @patch( + "redash.models.parameterized_query.dropdown_values", + return_value=[{ + "value": "baz" + }], + ) def test_validates_query_parameters(self, _): schema = [{"name": "bar", "type": "query", "queryId": 1}] query = ParameterizedQuery("foo {{bar}}", schema) @@ -204,9 +361,15 @@ def test_validates_date_range_parameters(self): schema = [{"name": "bar", "type": "date-range"}] query = ParameterizedQuery("foo {{bar.start}} {{bar.end}}", schema) - query.apply({"bar": {"start": "2000-01-01 12:00:00", "end": "2000-12-31 12:00:00"}}) + query.apply({ + "bar": { + "start": "2000-01-01 12:00:00", + "end": "2000-12-31 12:00:00" + } + }) - self.assertEqual("foo 2000-01-01 12:00:00 2000-12-31 12:00:00", query.text) + self.assertEqual("foo 2000-01-01 12:00:00 2000-12-31 12:00:00", + query.text) def test_raises_on_unexpected_param_types(self): schema = [{"name": "bar", "type": "burrito"}] @@ -233,28 +396,74 @@ def test_is_safe_if_not_expecting_any_parameters(self): self.assertTrue(query.is_safe) - @patch('redash.models.parameterized_query._load_result', return_value={ - "columns": [{"name": "id"}, {"name": "Name"}, {"name": "Value"}], - "rows": [{"id": 5, "Name": "John", "Value": "John Doe"}]}) + @patch( + "redash.models.parameterized_query._load_result", + return_value={ + "columns": [{ + "name": "id" + }, { + "name": "Name" + }, { + "name": "Value" + }], + "rows": [{ + "id": 5, + "Name": "John", + "Value": "John Doe" + }], + }, + ) def test_dropdown_values_prefers_name_and_value_columns(self, _): values = dropdown_values(1, None) self.assertEqual(values, [{"name": "John", "value": "John Doe"}]) - @patch('redash.models.parameterized_query._load_result', return_value={ - "columns": [{"name": "id"}, {"name": "fish"}, {"name": "poultry"}], - "rows": [{"fish": "Clown", "id": 5, "poultry": "Hen"}]}) + @patch( + "redash.models.parameterized_query._load_result", + return_value={ + "columns": [{ + "name": "id" + }, { + "name": "fish" + }, { + "name": "poultry" + }], + "rows": [{ + "fish": "Clown", + "id": 5, + "poultry": "Hen" + }], + }, + ) def test_dropdown_values_compromises_for_first_column(self, _): values = dropdown_values(1, None) self.assertEqual(values, [{"name": 5, "value": "5"}]) - @patch('redash.models.parameterized_query._load_result', return_value={ - "columns": [{"name": "ID"}, {"name": "fish"}, {"name": "poultry"}], - "rows": [{"fish": "Clown", "ID": 5, "poultry": "Hen"}]}) + @patch( + "redash.models.parameterized_query._load_result", + return_value={ + "columns": [{ + "name": "ID" + }, { + "name": "fish" + }, { + "name": "poultry" + }], + "rows": [{ + "fish": "Clown", + "ID": 5, + "poultry": "Hen" + }], + }, + ) def test_dropdown_supports_upper_cased_columns(self, _): values = dropdown_values(1, None) self.assertEqual(values, [{"name": 5, "value": "5"}]) - @patch('redash.models.Query.get_by_id_and_org', return_value=namedtuple('Query', 'data_source')(None)) - def test_dropdown_values_raises_when_query_is_detached_from_data_source(self, _): + @patch( + "redash.models.Query.get_by_id_and_org", + return_value=namedtuple("Query", "data_source")(None), + ) + def test_dropdown_values_raises_when_query_is_detached_from_data_source( + self, _): with pytest.raises(QueryDetachedFromDataSourceError): dropdown_values(1, None)