diff --git a/bin/version b/bin/version index b08a6e0657aa053844cac648f15b0bca8deabeeb..924dca62f05ddad0765152e78fe4395ae6b933ce 100755 --- a/bin/version +++ b/bin/version @@ -1,6 +1,6 @@ #!/usr/bin/env bash -VERSION="v0.36.0-snapshot" +VERSION="v0.36.1" # dynamically pull more interesting stuff from latest git commit HASH=$(git show-ref --head --hash=7 head) # first 7 letters of hash should be enough; that's what GitHub uses diff --git a/frontend/interfaces/underscore.js b/frontend/interfaces/underscore.js old mode 100644 new mode 100755 index 677ebe9be3ca46b6d16e7506d85f339fabaf342e..ca19feec88e981c74a274e2dbe65974dfb7ff723 --- a/frontend/interfaces/underscore.js +++ b/frontend/interfaces/underscore.js @@ -76,6 +76,11 @@ declare module "underscore" { iteratee: string | ((val: T, index: number) => any), ): { [key: string]: T[] }; + declare function indexBy<T>( + a: Array<T>, + iteratee: string | ((val: T, index: number) => any), + ): { [key: string]: T }; + declare function min<T>(a: Array<T> | { [key: any]: T }): T; declare function max<T>(a: Array<T> | { [key: any]: T }): T; diff --git a/frontend/src/metabase-lib/lib/Dimension.js b/frontend/src/metabase-lib/lib/Dimension.js index eebfa174a71eb9ae1c3c1e23bcd0cbb0266cb586..e1f6c6f16b0d55f89c43611cf93b0547fc8fa752 100644 --- a/frontend/src/metabase-lib/lib/Dimension.js +++ b/frontend/src/metabase-lib/lib/Dimension.js @@ -536,10 +536,6 @@ export class FieldLiteralDimension extends FieldDimension { base_type: this._args[1], // HACK: need to thread the query through to this fake Field query: this._query, - filter_operators: [{ name: "=", verboseName: t`Is`, fields: [] }], - filter_operators_lookup: { - "=": { name: "=", verboseName: t`Is`, fields: [] }, - }, }); } } diff --git a/frontend/src/metabase/admin/datamodel/components/FieldRemapping.jsx b/frontend/src/metabase/admin/datamodel/components/FieldRemapping.jsx index f90e16f8a8c792f146ebf354536669d8251a4159..abad37e04532abcf0e057e8228bc31a3ce74c0a1 100644 --- a/frontend/src/metabase/admin/datamodel/components/FieldRemapping.jsx +++ b/frontend/src/metabase/admin/datamodel/components/FieldRemapping.jsx @@ -66,7 +66,9 @@ export default class FieldRemapping extends React.Component { // (for a field without user-defined remappings, every key of `field.remappings` has value `undefined`) const hasMappableNumeralValues = field.remapping.size > 0 && - [...field.remapping.keys()].every(key => typeof key === "number"); + [...field.remapping.keys()].every( + key => typeof key === "number" || key === null, + ); return [ MAP_OPTIONS.original, @@ -353,6 +355,8 @@ export class ValueRemappings extends React.Component { const mappedString = mappedOrUndefined !== undefined ? mappedOrUndefined.toString() + : original === null + ? "null" : original.toString(); return [original, mappedString]; diff --git a/frontend/src/metabase/admin/people/components/GroupSelect.jsx b/frontend/src/metabase/admin/people/components/GroupSelect.jsx index 8ff4ea94acfb09601b3776b3d99165ada744f495..533c1af78094f66d1694ade8edf37877095c6cb8 100644 --- a/frontend/src/metabase/admin/people/components/GroupSelect.jsx +++ b/frontend/src/metabase/admin/people/components/GroupSelect.jsx @@ -1,6 +1,11 @@ import React from "react"; +import { t } from "ttag"; -import CheckBox from "metabase/components/CheckBox"; +import Icon from "metabase/components/Icon"; +import Select from "metabase/components/Select"; +import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; + +import GroupSummary from "./GroupSummary"; import { isDefaultGroup, @@ -9,69 +14,61 @@ import { getGroupColor, getGroupNameLocalized, } from "metabase/lib/groups"; -import cx from "classnames"; -import _ from "underscore"; -export const GroupOption = ({ - group, - selectedGroups = {}, +export const GroupSelect = ({ + groups, + selectedGroupIds = new Set(), onGroupChange, - isDisabled = false, + isCurrentUser = false, + emptyListMessage = t`No groups`, }) => { - const disabled = isDisabled || !canEditMembership(group); - const selected = isDefaultGroup(group) || selectedGroups[group.id]; - return ( - <div - className={cx("GroupOption flex align-center p1 px2", { - "cursor-pointer": !disabled, - })} - onClick={() => !disabled && onGroupChange(group, !selected)} - > - <span className={cx("pr1", getGroupColor(group), { disabled })}> - <CheckBox checked={selected} size={18} /> + const triggerElement = ( + <div className="flex align-center"> + <span className="mr1 text-medium"> + <GroupSummary groups={groups} selectedGroupIds={selectedGroupIds} /> </span> - {getGroupNameLocalized(group)} + <Icon className="text-light" name="chevrondown" size={10} /> </div> ); -}; -export const GroupSelect = ({ - groups, - selectedGroups, - onGroupChange, - isCurrentUser, -}) => { + if (groups.length === 0) { + return ( + <PopoverWithTrigger triggerElement={triggerElement}> + <span className="p1">{emptyListMessage}</span> + </PopoverWithTrigger> + ); + } const other = groups.filter(g => !isAdminGroup(g) && !isDefaultGroup(g)); - const adminGroup = _.find(groups, isAdminGroup); - const defaultGroup = _.find(groups, isDefaultGroup); + const adminGroup = groups.find(isAdminGroup); + const defaultGroup = groups.find(isDefaultGroup); + const topGroups = [defaultGroup, adminGroup].filter(g => g != null); + return ( - <div className="GroupSelect scroll-y py1"> - {adminGroup && ( - <GroupOption - group={adminGroup} - selectedGroups={selectedGroups} - onGroupChange={onGroupChange} - isDisabled={isCurrentUser} - /> - )} - {defaultGroup && ( - <GroupOption - group={defaultGroup} - selectedGroups={selectedGroups} - onGroupChange={onGroupChange} - /> - )} - {other.length > 0 && (defaultGroup || adminGroup) && ( - <div key="divider" className="border-bottom pb1 mb1" /> - )} - {other.map(group => ( - <GroupOption - group={group} - selectedGroups={selectedGroups} - onGroupChange={onGroupChange} - /> - ))} - </div> + <Select + triggerElement={triggerElement} + onChange={({ target: { value } }) => { + groups + .filter( + // find the differing groups between the new `value` on previous `selectedGroupIds` + group => + selectedGroupIds.includes(group.id) ^ value.includes(group.id), + ) + .forEach(group => onGroupChange(group, value.includes(group.id))); + }} + optionDisabledFn={group => + (isAdminGroup(group) && isCurrentUser) || !canEditMembership(group) + } + optionValueFn={group => group.id} + optionNameFn={getGroupNameLocalized} + optionClassNameFn={getGroupColor} + value={selectedGroupIds} + sections={ + topGroups.length > 0 + ? [{ items: topGroups }, { items: other, name: t`Groups` }] + : [{ items: other }] + } + multiple + /> ); }; diff --git a/frontend/src/metabase/admin/people/components/GroupSummary.jsx b/frontend/src/metabase/admin/people/components/GroupSummary.jsx index a591e48be84c4645c4b8a7445107e3b609b02bea..d69726f60ddb27f07624d6f7cd59fef3bb0b5774 100644 --- a/frontend/src/metabase/admin/people/components/GroupSummary.jsx +++ b/frontend/src/metabase/admin/people/components/GroupSummary.jsx @@ -1,15 +1,15 @@ import React from "react"; -import _ from "underscore"; import { t, ngettext, msgid } from "ttag"; import { isAdminGroup, isDefaultGroup } from "metabase/lib/groups"; -const GroupSummary = ({ groups, selectedGroups }) => { - const adminGroup = _.find(groups, isAdminGroup); +const GroupSummary = ({ groups, selectedGroupIds }) => { + const adminGroup = groups.find(isAdminGroup); const otherGroups = groups.filter( - g => selectedGroups[g.id] && !isAdminGroup(g) && !isDefaultGroup(g), + g => + selectedGroupIds.includes(g.id) && !isAdminGroup(g) && !isDefaultGroup(g), ); - if (adminGroup && selectedGroups[adminGroup.id]) { + if (adminGroup && selectedGroupIds.includes(adminGroup.id)) { return ( <span> <span className="text-purple">{t`Admin`}</span> diff --git a/frontend/src/metabase/admin/people/components/UserGroupSelect.jsx b/frontend/src/metabase/admin/people/components/UserGroupSelect.jsx index 537a1a229f5fd42636ef45edf0780244fdc97519..759742e0eaa706aab286d26d88b76241920864fc 100644 --- a/frontend/src/metabase/admin/people/components/UserGroupSelect.jsx +++ b/frontend/src/metabase/admin/people/components/UserGroupSelect.jsx @@ -1,35 +1,10 @@ /* eslint "react/prop-types": "warn" */ import React, { Component } from "react"; import PropTypes from "prop-types"; -import cx from "classnames"; -import Icon from "metabase/components/Icon"; -import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; -import CheckBox from "metabase/components/CheckBox"; import LoadingSpinner from "metabase/components/LoadingSpinner"; import GroupSelect from "./GroupSelect"; -import GroupSummary from "./GroupSummary"; - -const GroupOption = ({ name, color, selected, disabled, onChange }) => ( - <div - className={cx("flex align-center p1 px2", { "cursor-pointer": !disabled })} - onClick={() => !disabled && onChange(!selected)} - > - <span className={cx("pr1", color, { disabled })}> - <CheckBox checked={selected} size={18} /> - </span> - {name} - </div> -); - -GroupOption.propTypes = { - name: PropTypes.string, - color: PropTypes.string, - selected: PropTypes.bool, - disabled: PropTypes.bool, - onChange: PropTypes.func, -}; export default class UserGroupSelect extends Component { static propTypes = { @@ -70,28 +45,16 @@ export default class UserGroupSelect extends Component { }); } }; - + const selectedGroupIds = Object.values(user.memberships).map( + m => m.group_id, + ); return ( - <PopoverWithTrigger - ref="popover" - triggerElement={ - <div className="flex align-center"> - <span className="mr1 text-medium"> - <GroupSummary groups={groups} selectedGroups={user.memberships} /> - </span> - <Icon className="text-light" name="chevrondown" size={10} /> - </div> - } - triggerClasses="AdminSelectBorderless py1" - sizeToFit - > - <GroupSelect - groups={groups} - selectedGroups={user.memberships} - onGroupChange={changeMembership} - isCurrentUser={isCurrentUser} - /> - </PopoverWithTrigger> + <GroupSelect + groups={groups} + selectedGroupIds={selectedGroupIds} + onGroupChange={changeMembership} + isCurrentUser={isCurrentUser} + /> ); } } diff --git a/frontend/src/metabase/admin/permissions/containers/CollectionPermissionsModal.jsx b/frontend/src/metabase/admin/permissions/containers/CollectionPermissionsModal.jsx index 230ee9bc7522c34237a97fa2c56742a825aad8f0..cbc9a695ecb6ed7efca16c4688039b2a903d233b 100644 --- a/frontend/src/metabase/admin/permissions/containers/CollectionPermissionsModal.jsx +++ b/frontend/src/metabase/admin/permissions/containers/CollectionPermissionsModal.jsx @@ -1,15 +1,16 @@ import React, { Component } from "react"; import { connect } from "react-redux"; import { t } from "ttag"; +import _ from "underscore"; import ModalContent from "metabase/components/ModalContent"; import Button from "metabase/components/Button"; import Link from "metabase/components/Link"; import PermissionsGrid from "../components/PermissionsGrid"; -import fitViewport from "metabase/hoc/FitViewPort"; import { CollectionsApi } from "metabase/services"; import Collections from "metabase/entities/collections"; +import SnippetCollections from "metabase/entities/snippet-collections"; import { getCollectionsPermissionsGrid, @@ -18,44 +19,77 @@ import { } from "../selectors"; import { initialize, updatePermission, savePermissions } from "../permissions"; +const getCollectionEntity = props => + props.namespace === "snippets" ? SnippetCollections : Collections; + const mapStateToProps = (state, props) => { + const { collectionId } = props.params; return { grid: getCollectionsPermissionsGrid(state, { - collectionId: props.params.collectionId, + collectionId, singleCollectionMode: true, + namespace: props.namespace, }), isDirty: getIsDirty(state, props), diff: getDiff(state, props), + collection: getCollectionEntity(props).selectors.getObject(state, { + entityId: collectionId, + }), }; }; -const mapDispatchToProps = { - initialize, - loadCollections: Collections.actions.fetchList, - onUpdatePermission: updatePermission, - onSave: savePermissions, -}; +const mapDispatchToProps = (dispatch, props) => + _.mapObject( + { + initialize, + loadCollections: getCollectionEntity(props).actions.fetchList, + onUpdatePermission: updatePermission, + onSave: savePermissions, + }, + f => (...args) => dispatch(f(...args)), + ); @connect( mapStateToProps, mapDispatchToProps, ) -@fitViewport export default class CollectionPermissionsModal extends Component { componentWillMount() { - this.props.initialize(CollectionsApi.graph, CollectionsApi.updateGraph); - this.props.loadCollections(); + const { namespace, loadCollections, initialize } = this.props; + initialize( + () => CollectionsApi.graph({ namespace }), + graph => CollectionsApi.updateGraph({ ...graph, namespace }), + ); + loadCollections(); } render() { - const { grid, onUpdatePermission, isDirty, onClose, onSave } = this.props; + const { + grid, + onUpdatePermission, + isDirty, + onClose, + onSave, + namespace, + collection, + } = this.props; return ( <ModalContent - title={t`Permissions for this collection`} + title={ + collection && collection.name + ? t`Permissions for ${collection.name}` + : namespace === "snippets" + ? t`Permissions for this folder` + : t`Permissions for this collection` + } onClose={onClose} footer={[ - <Link className="link" to="/admin/permissions/collections"> - {t`See all collection permissions`} - </Link>, + ...(namespace === "snippets" + ? [] + : [ + <Link className="link" to="/admin/permissions/collections"> + {t`See all collection permissions`} + </Link>, + ]), <Button onClick={onClose}>{t`Cancel`}</Button>, <Button primary diff --git a/frontend/src/metabase/admin/permissions/containers/TogglePropagateAction.jsx b/frontend/src/metabase/admin/permissions/containers/TogglePropagateAction.jsx index ee24f45ad4f4094c53cea7e0b9d918ce42ab6d02..791a2967ff1942a2696c84a5c7788c7ab6377a65 100644 --- a/frontend/src/metabase/admin/permissions/containers/TogglePropagateAction.jsx +++ b/frontend/src/metabase/admin/permissions/containers/TogglePropagateAction.jsx @@ -2,8 +2,6 @@ import React from "react"; import { connect } from "react-redux"; -import { t } from "ttag"; - import { getPropagatePermissions } from "../selectors"; import { setPropagatePermissions } from "../permissions"; @@ -19,15 +17,15 @@ const mapDispatchToProps = { const TogglePropagateAction = connect( mapStateToProps, mapDispatchToProps, -)(({ propagate, setPropagatePermissions }) => ( +)(({ propagate, setPropagatePermissions, message }) => ( <div className="flex align-center bg-medium px2 py1 cursor-pointer" onClick={() => setPropagatePermissions(!propagate)} > - <span className="mr2 text-small">{t`Also change sub-collections`}</span> + <span className="mr2 text-small">{message}</span> <Toggle small value={propagate} /> </div> )); // eslint-disable-next-line react/display-name -export default () => <TogglePropagateAction />; +export default props => <TogglePropagateAction {...props} />; diff --git a/frontend/src/metabase/admin/permissions/selectors.js b/frontend/src/metabase/admin/permissions/selectors.js index 351a13ca325fa0d4f10e03b7c3519b30aa9d6efb..fc99a617367ddcda1cd6ad25c43b94018c058deb 100644 --- a/frontend/src/metabase/admin/permissions/selectors.js +++ b/frontend/src/metabase/admin/permissions/selectors.js @@ -292,6 +292,24 @@ const OPTION_COLLECTION_READ = { tooltip: t`Can view items in this collection`, }; +const OPTION_SNIPPET_COLLECTION_WRITE = { + ...OPTION_COLLECTION_WRITE, + title: t`Grant Edit access`, + tooltip: t`Can modify snippets in this folder`, +}; + +const OPTION_SNIPPET_COLLECTION_READ = { + ...OPTION_COLLECTION_READ, + title: t`Grant View access`, + tooltip: t`Can insert and use snippets in this folder, but can't edit the SQL they contain`, +}; + +const OPTION_SNIPPET_COLLECTION_NONE = { + ...OPTION_NONE, + title: t`Revoke access`, + tooltip: t`Can't view or insert snippets in this folder`, +}; + export const getTablesPermissionsGrid = createSelector( getMetadata, getGroups, @@ -683,6 +701,7 @@ export const getDatabasesPermissionsGrid = createSelector( ); import Collections from "metabase/entities/collections"; +import SnippetCollections from "metabase/entities/snippet-collections"; const getCollectionId = (state, props) => props && props.collectionId; @@ -691,9 +710,17 @@ const getSingleCollectionPermissionsMode = (state, props) => const permissionsCollectionFilter = collection => !collection.is_personal; +const getNamespace = (state, props) => props.namespace; + +const getExpandedCollectionsById = (state, props) => + (props.namespace === "snippets" + ? SnippetCollections + : Collections + ).selectors.getExpandedCollectionsById(state, props); + const getCollections = createSelector( [ - Collections.selectors.getExpandedCollectionsById, + getExpandedCollectionsById, getCollectionId, getSingleCollectionPermissionsMode, ], @@ -726,11 +753,13 @@ export const getCollectionsPermissionsGrid = createSelector( getGroups, getPermissions, getPropagatePermissions, + getNamespace, ( collections, groups: Array<GroupType>, permissions: GroupsPermissions, propagatePermissions: boolean, + namespace: string, ) => { if (!groups || groups.length === 0 || !permissions || !collections) { return null; @@ -764,18 +793,28 @@ export const getCollectionsPermissionsGrid = createSelector( access: { header: t`Collection Access`, options(groupId, entityId) { - return [ - OPTION_COLLECTION_WRITE, - OPTION_COLLECTION_READ, - OPTION_NONE, - ]; + return namespace === "snippets" + ? [ + OPTION_SNIPPET_COLLECTION_WRITE, + OPTION_SNIPPET_COLLECTION_READ, + OPTION_SNIPPET_COLLECTION_NONE, + ] + : [OPTION_COLLECTION_WRITE, OPTION_COLLECTION_READ, OPTION_NONE]; }, actions(groupId, { collectionId }) { const collection = _.findWhere(collections, { id: collectionId, }); if (collection && collection.children.length > 0) { - return [TogglePropagateAction]; + return [ + () => + TogglePropagateAction({ + message: + namespace === "snippets" + ? t`Also change sub-folders` + : t`Also change sub-collections`, + }), + ]; } else { return []; } diff --git a/frontend/src/metabase/admin/settings/components/SettingsSlackForm.jsx b/frontend/src/metabase/admin/settings/components/SettingsSlackForm.jsx index 7c29b9ad6ee5e4bbe651c85c401aab68eda67828..fa1a9b24ddb89654075064b5fb9f29e4d254cb9f 100644 --- a/frontend/src/metabase/admin/settings/components/SettingsSlackForm.jsx +++ b/frontend/src/metabase/admin/settings/components/SettingsSlackForm.jsx @@ -237,7 +237,7 @@ export default class SettingsSlackForm extends Component { <form noValidate> <div className="px2" style={{ maxWidth: "585px" }}> <h1> - Metabase + {t`Metabase`} <RetinaImage className="mx1" src="app/assets/img/slack_emoji.png" diff --git a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget.jsx b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget.jsx index ff90d0d748e76cd24c8bb714274b2182009d396a..b66396aa20891142309454d3d9676ad8d77d8577 100644 --- a/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget.jsx +++ b/frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget.jsx @@ -6,11 +6,8 @@ import { ModalFooter } from "metabase/components/ModalContent"; import AdminContentTable from "metabase/components/AdminContentTable"; import Button from "metabase/components/Button"; import GroupSelect from "metabase/admin/people/components/GroupSelect"; -import GroupSummary from "metabase/admin/people/components/GroupSummary"; -import Icon from "metabase/components/Icon"; import LoadingSpinner from "metabase/components/LoadingSpinner"; import Modal from "metabase/components/Modal"; -import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; import { t } from "ttag"; import { PermissionsApi, SettingsApi } from "metabase/services"; import { isSpecialGroup } from "metabase/lib/groups"; @@ -303,35 +300,13 @@ class MappingGroupSelect extends React.Component { return <LoadingSpinner />; } - const selected = selectedGroups.reduce( - (g, id) => ({ ...g, [id]: true }), - {}, - ); - return ( - <PopoverWithTrigger - ref="popover" - triggerElement={ - <div className="flex align-center"> - <span className="mr1 text-medium"> - <GroupSummary groups={groups} selectedGroups={selected} /> - </span> - <Icon className="text-light" name="chevrondown" size={10} /> - </div> - } - triggerClasses="AdminSelectBorderless py1" - sizeToFit - > - {groups.length > 0 ? ( - <GroupSelect - groups={groups} - selectedGroups={selected} - onGroupChange={onGroupChange} - /> - ) : ( - <span className="p1">{t`No mappable groups`}</span> - )} - </PopoverWithTrigger> + <GroupSelect + groups={groups} + selectedGroupIds={selectedGroups} + onGroupChange={onGroupChange} + emptyListMessage={t`No mappable groups`} + /> ); } } diff --git a/frontend/src/metabase/components/Select.jsx b/frontend/src/metabase/components/Select.jsx index ed5fc9db2f9f3415c2cb8eb6937e97e12e873847..35f8031bc41840bae9c06c99c9b84a18e2d48d2d 100644 --- a/frontend/src/metabase/components/Select.jsx +++ b/frontend/src/metabase/components/Select.jsx @@ -36,6 +36,7 @@ export default class Select extends Component { // PopoverWithTrigger props isInitiallyOpen: PropTypes.bool, + triggerElement: PropTypes.any, // SelectButton props buttonProps: PropTypes.object, @@ -51,6 +52,7 @@ export default class Select extends Component { optionSectionFn: PropTypes.func, optionDisabledFn: PropTypes.func, optionIconFn: PropTypes.func, + optionClassNameFn: PropTypes.func, }; static defaultProps = { @@ -187,20 +189,22 @@ export default class Select extends Component { <PopoverWithTrigger ref={ref => (this._popover = ref)} triggerElement={ - <SelectButton - className="full-width" - hasValue={selectedNames.length > 0} - {...buttonProps} - > - {selectedNames.length > 0 - ? selectedNames.map((name, index) => ( - <span key={index}> - {name} - {index < selectedNames.length - 1 ? ", " : ""} - </span> - )) - : placeholder} - </SelectButton> + this.props.triggerElement || ( + <SelectButton + className="flex-full" + hasValue={selectedNames.length > 0} + {...buttonProps} + > + {selectedNames.length > 0 + ? selectedNames.map((name, index) => ( + <span key={index}> + {name} + {index < selectedNames.length - 1 ? ", " : ""} + </span> + )) + : placeholder} + </SelectButton> + ) } triggerClasses={cx("flex", className)} isInitiallyOpen={isInitiallyOpen} @@ -216,6 +220,7 @@ export default class Select extends Component { itemIsSelected={this.itemIsSelected} itemIsClickable={this.itemIsClickable} renderItemName={this.props.optionNameFn} + getItemClassName={this.props.optionClassNameFn} renderItemDescription={this.props.optionDescriptionFn} renderItemIcon={this.renderItemIcon} onChange={this.handleChange} diff --git a/frontend/src/metabase/components/form/FormWidget.jsx b/frontend/src/metabase/components/form/FormWidget.jsx index ba77d04dffbd630590b0e3128de4f8e9791e2c6d..4898d6cde9c3049c3b33e838c2d0ed08679c88bf 100644 --- a/frontend/src/metabase/components/form/FormWidget.jsx +++ b/frontend/src/metabase/components/form/FormWidget.jsx @@ -10,6 +10,7 @@ import FormSelectWidget from "./widgets/FormSelectWidget"; import FormNumericInputWidget from "./widgets/FormNumericInputWidget"; import FormToggleWidget from "./widgets/FormToggleWidget"; import FormCollectionWidget from "./widgets/FormCollectionWidget"; +import FormSnippetCollectionWidget from "./widgets/FormSnippetCollectionWidget"; import FormHiddenWidget from "./widgets/FormHiddenWidget"; import FormTextFileWidget from "./widgets/FormTextFileWidget"; @@ -24,6 +25,7 @@ const WIDGETS = { integer: FormNumericInputWidget, boolean: FormToggleWidget, collection: FormCollectionWidget, + snippetCollection: FormSnippetCollectionWidget, hidden: FormHiddenWidget, textFile: FormTextFileWidget, }; diff --git a/frontend/src/metabase/components/form/widgets/FormGroupsWidget.jsx b/frontend/src/metabase/components/form/widgets/FormGroupsWidget.jsx index 517855e0792faf0a5b75fef2b0020b19b09155ca..080a228c49dfaad1b2645c2ff66c2ca5f05f4579 100644 --- a/frontend/src/metabase/components/form/widgets/FormGroupsWidget.jsx +++ b/frontend/src/metabase/components/form/widgets/FormGroupsWidget.jsx @@ -1,9 +1,6 @@ import React from "react"; -import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; import GroupSelect from "metabase/admin/people/components/GroupSelect"; -import GroupSummary from "metabase/admin/people/components/GroupSummary"; -import SelectButton from "metabase/components/SelectButton"; import Toggle from "metabase/components/Toggle"; import Group from "metabase/entities/groups"; @@ -26,8 +23,6 @@ const FormGroupsWidget = ({ field: { value, onChange }, groups }) => { } const selection = new Set(value); - // NOTE: for legacy selectedGroups prop on GroupSummary and GroupSelect. Prefer using a Set - const selectedGroups = _.object(value.map(id => [id, true])); function onGroupChange(group, selected) { const newSelection = new Set(selection); @@ -50,20 +45,11 @@ const FormGroupsWidget = ({ field: { value, onChange }, groups }) => { ); return hasNonAdminEditableGroups ? ( - <PopoverWithTrigger - sizeToFit - triggerElement={ - <SelectButton> - <GroupSummary groups={groups} selectedGroups={selectedGroups} /> - </SelectButton> - } - > - <GroupSelect - groups={visibleGroups} - selectedGroups={selectedGroups} - onGroupChange={onGroupChange} - /> - </PopoverWithTrigger> + <GroupSelect + groups={visibleGroups} + selectedGroupIds={value} + onGroupChange={onGroupChange} + /> ) : hadAdminGroup ? ( <div className="flex align-center"> <Toggle diff --git a/frontend/src/metabase/components/form/widgets/FormSnippetCollectionWidget.jsx b/frontend/src/metabase/components/form/widgets/FormSnippetCollectionWidget.jsx new file mode 100644 index 0000000000000000000000000000000000000000..62bf6474d2a1121f145301937eb9d310702f8d8d --- /dev/null +++ b/frontend/src/metabase/components/form/widgets/FormSnippetCollectionWidget.jsx @@ -0,0 +1,21 @@ +import React from "react"; + +import ItemSelect from "metabase/containers/ItemSelect"; +import CollectionPicker from "metabase/containers/CollectionPicker"; +import SnippetCollections from "metabase/entities/snippet-collections"; + +const CollectionSelect = ItemSelect( + CollectionPicker, + SnippetCollections.Name, + "collection", +); + +const FormSnippetCollectionWidget = ({ field }) => ( + <CollectionSelect + entity={SnippetCollections} + showSearch={false} // seems that search endpoint doesn't support namespace yet + {...field} + value={field.value || "root"} // needed so SnippetCollections.Name finds the right collection + /> +); +export default FormSnippetCollectionWidget; diff --git a/frontend/src/metabase/containers/CollectionName.jsx b/frontend/src/metabase/containers/CollectionName.jsx index aa71728318e666889af1c1a6fcfb4bb4bf49678e..ca0b857b308ad1fa775cbdf100b519957cbb09b6 100644 --- a/frontend/src/metabase/containers/CollectionName.jsx +++ b/frontend/src/metabase/containers/CollectionName.jsx @@ -2,13 +2,13 @@ import React from "react"; import Collection, { ROOT_COLLECTION } from "metabase/entities/collections"; -const CollectionName = ({ collectionId }) => { - if (collectionId === undefined || isNaN(collectionId)) { +const CollectionName = ({ id }) => { + if (id === undefined || isNaN(id)) { return null; - } else if (collectionId === "root" || collectionId === null) { + } else if (id === "root" || id === null) { return <span>{ROOT_COLLECTION.name}</span>; } else { - return <Collection.Name id={collectionId} />; + return <Collection.Name id={id} />; } }; diff --git a/frontend/src/metabase/containers/Form.jsx b/frontend/src/metabase/containers/Form.jsx index 71c65749a9676b8e39d2a8d7a8258aaed222e164..99b8481326184dde1ce45ad4d494d3a6ff200121 100644 --- a/frontend/src/metabase/containers/Form.jsx +++ b/frontend/src/metabase/containers/Form.jsx @@ -31,7 +31,8 @@ type FormFieldType = | "text" | "color" | "hidden" - | "collection"; + | "collection" + | "snippetCollection"; type FormValue = any; type FormError = string; diff --git a/frontend/src/metabase/containers/ItemPicker.jsx b/frontend/src/metabase/containers/ItemPicker.jsx index 1833028730078c400ba349288037ad80243ef4db..7de37ddeb956ca7fca9cf5e5ef7a4054468b5beb 100644 --- a/frontend/src/metabase/containers/ItemPicker.jsx +++ b/frontend/src/metabase/containers/ItemPicker.jsx @@ -24,11 +24,16 @@ const COLLECTION_ICON_COLOR = color("text-light"); const isRoot = collection => collection.id === "root" || collection.id == null; @entityListLoader({ - entityType: "collections", + entityType: (state, props) => { + return props.entity ? props.entity.name : "collections"; + }, loadingAndErrorWrapper: false, }) @connect((state, props) => ({ - collectionsById: Collections.selectors.getExpandedCollectionsById(state), + collectionsById: ( + props.entity || Collections + ).selectors.getExpandedCollectionsById(state), + getCollectionIcon: (props.entity || Collections).objectSelectors.getIcon, })) export default class ItemPicker extends React.Component { constructor(props) { @@ -46,6 +51,7 @@ export default class ItemPicker extends React.Component { // number = non-root collection id value: PropTypes.number, types: PropTypes.array, + showSearch: PropTypes.boolean, }; // returns a list of "crumbs" starting with the "root" collection @@ -70,7 +76,15 @@ export default class ItemPicker extends React.Component { } render() { - const { value, onChange, collectionsById, style, className } = this.props; + const { + value, + onChange, + collectionsById, + getCollectionIcon, + style, + className, + showSearch = true, + } = this.props; const { parentId, searchMode, searchString } = this.state; const models = new Set(this.props.models); @@ -131,11 +145,13 @@ export default class ItemPicker extends React.Component { ) : ( <Box pb={1} mb={2} className="border-bottom flex align-center"> <Breadcrumbs crumbs={crumbs} /> - <Icon - name="search" - className="ml-auto pl2 text-light text-medium-hover cursor-pointer" - onClick={() => this.setState({ searchMode: true })} - /> + {showSearch && ( + <Icon + name="search" + className="ml-auto pl2 text-light text-medium-hover cursor-pointer" + onClick={() => this.setState({ searchMode: true })} + /> + )} </Box> )} <Box className="scroll-y"> @@ -158,7 +174,7 @@ export default class ItemPicker extends React.Component { item={collection} name={collection.name} color={COLLECTION_ICON_COLOR} - icon="all" + icon={getCollectionIcon(collection)} selected={canSelect && isSelected(collection)} canSelect={canSelect} hasChildren={hasChildren} diff --git a/frontend/src/metabase/containers/ItemSelect.jsx b/frontend/src/metabase/containers/ItemSelect.jsx index 1cbd561b3f01a3f240aa09a8a3c50c22c711af4b..a3ee17c3dfb3ddfc9caaf2be3963f2f31e31be05 100644 --- a/frontend/src/metabase/containers/ItemSelect.jsx +++ b/frontend/src/metabase/containers/ItemSelect.jsx @@ -60,7 +60,7 @@ export default (PickerComponent, NameComponent, type) => triggerElement={ <SelectButton style={style}> {value !== undefined && value !== "" ? ( - <NameComponent collectionId={value} /> + <NameComponent id={value} /> ) : ( placeholder )} diff --git a/frontend/src/metabase/css/components/list.css b/frontend/src/metabase/css/components/list.css index a72c9298561247f498006860e8449b96f78123e1..6f7f7a07df2d5074dc687a00e71bc471d0219260 100644 --- a/frontend/src/metabase/css/components/list.css +++ b/frontend/src/metabase/css/components/list.css @@ -45,7 +45,7 @@ margin-bottom: 2px; } .List-item:not(.List-item--disabled):hover, -.List-item--selected { +.List-item--selected:not(.List-item--disabled) { background-color: currentColor; border-color: color(var(--color-accent2) alpha(-80%)); /*color: white;*/ @@ -59,7 +59,7 @@ color: var(--color-text-medium); } .List-item:not(.List-item--disabled):hover .List-item-title, -.List-item--selected .List-item-title { +.List-item--selected:not(.List-item--disabled) .List-item-title { color: white; } @@ -73,12 +73,12 @@ color: var(--color-text-dark); } .List-item:not(.List-item--disabled):hover .List-item-description, -.List-item--selected .List-item-description { +.List-item--selected:not(.List-item--disabled) .List-item-description { color: rgba(255, 255, 255, 0.5); } /* LIST ITEM ICON */ .List-item:not(.List-item--disabled):hover .Icon, -.List-item--selected .Icon { +.List-item--selected:not(.List-item--disabled) .Icon { color: white !important; } diff --git a/frontend/src/metabase/dashboard/dashboard.js b/frontend/src/metabase/dashboard/dashboard.js index c457c29d3b4de0be1e277b9c63475bd18531b36d..d925fa01bf4f55f9ef2589630e556af0aae11301 100644 --- a/frontend/src/metabase/dashboard/dashboard.js +++ b/frontend/src/metabase/dashboard/dashboard.js @@ -1167,7 +1167,7 @@ const loadMetadataForDashboard = dashCards => (dispatch, getState) => { const metadata = getMetadata(getState()); const queries = dashCards - .filter(dc => !isVirtualDashCard(dc)) + .filter(dc => !isVirtualDashCard(dc) && dc.card.dataset_query) // exclude text cards and queries without perms .flatMap(dc => [dc.card].concat(dc.series || [])) .map(card => new Question(card, metadata).query()); diff --git a/frontend/src/metabase/entities/collections.js b/frontend/src/metabase/entities/collections.js index 7e8d08aab238d109c406c1af5923e3ce7b825e10..668595b7a390eaad33ce762756a783d1a5897276 100644 --- a/frontend/src/metabase/entities/collections.js +++ b/frontend/src/metabase/entities/collections.js @@ -195,7 +195,7 @@ type ExpandedCollection = { // given list of collections with { id, name, location } returns a map of ids to // expanded collection objects like { id, name, location, path, children } // including a root collection -function getExpandedCollectionsById( +export function getExpandedCollectionsById( collections: Collection[], userPersonalCollectionId: ?CollectionId, ): { [key: PseudoCollectionId]: ExpandedCollection } { diff --git a/frontend/src/metabase/entities/containers/EntityType.jsx b/frontend/src/metabase/entities/containers/EntityType.jsx index fbc615621be431253c1cba5ec26a162f45e54f2f..50ae1e085c7096f5c07f9797ef91ea2c6ba43799 100644 --- a/frontend/src/metabase/entities/containers/EntityType.jsx +++ b/frontend/src/metabase/entities/containers/EntityType.jsx @@ -10,7 +10,12 @@ export default (entityType?: string) => ( const mapStateToProps = (state, props) => ({ entityDef: // dynamic require due to dependency load order issues - require("metabase/entities")[entityType || props.entityType], + require("metabase/entities")[ + entityType || + (typeof props.entityType === "function" + ? props.entityType(state, props) + : props.entityType) + ], }); return connect(mapStateToProps)( class extends React.Component { diff --git a/frontend/src/metabase/entities/fields.js b/frontend/src/metabase/entities/fields.js index 0d2adf1b10ca57789a58f003fe3040bf64a3439d..794cecbc77abac7d6aa0e338da01b78c39b9fc6c 100644 --- a/frontend/src/metabase/entities/fields.js +++ b/frontend/src/metabase/entities/fields.js @@ -1,9 +1,12 @@ import { createEntity } from "metabase/lib/entities"; import { + compose, + withAction, + withCachedDataAndRequestState, + withNormalize, handleActions, createAction, createThunkAction, - fetchData, updateData, } from "metabase/lib/redux"; import { normalize } from "normalizr"; @@ -38,7 +41,7 @@ export const ADD_REMAPPINGS = "metabase/entities/fields/ADD_REMAPPINGS"; export const ADD_PARAM_VALUES = "metabase/entities/fields/ADD_PARAM_VALUES"; export const ADD_FIELDS = "metabase/entities/fields/ADD_FIELDS"; -export default createEntity({ +const Fields = createEntity({ name: "fields", path: "/api/field", schema: FieldSchema, @@ -64,18 +67,19 @@ export default createEntity({ // ACTION CREATORS objectActions: { - fetchFieldValues: createThunkAction( - FETCH_FIELD_VALUES, - ({ id }, reload) => (dispatch, getState) => - fetchData({ - dispatch, - getState, - requestStatePath: ["entities", "fields", id, "values"], - existingStatePath: ["entities", "fields", id, "values"], - getData: () => MetabaseApi.field_values({ fieldId: id }), - reload, - }), - ), + fetchFieldValues: compose( + withAction(FETCH_FIELD_VALUES), + withCachedDataAndRequestState( + ({ id }) => [...Fields.getObjectStatePath(id)], + ({ id }) => [...Fields.getObjectStatePath(id), "values"], + ), + withNormalize(FieldSchema), + )(({ id: fieldId }) => async (dispatch, getState) => { + const { field_id: id, values } = await MetabaseApi.field_values({ + fieldId, + }); + return { id, values }; + }), // Docstring from m.api.field: // Update the human-readable values for a `Field` whose special type is @@ -139,16 +143,6 @@ export default createEntity({ reducer: handleActions( { - [FETCH_FIELD_VALUES]: { - next: (state, { payload: fieldValues }) => - fieldValues - ? assocIn( - state, - [fieldValues.field_id, "values"], - fieldValues.values, - ) - : state, - }, [ADD_PARAM_VALUES]: { next: (state, { payload: paramValues }) => { for (const fieldValues of Object.values(paramValues)) { @@ -201,3 +195,5 @@ export default createEntity({ ].filter(f => f), }, }); + +export default Fields; diff --git a/frontend/src/metabase/entities/index.js b/frontend/src/metabase/entities/index.js index 52127c5c81e95c3a1e2e9f0c934c279464be51e3..4cb43a26cc1eeacf14a6f27a6e63a327422f7f6c 100644 --- a/frontend/src/metabase/entities/index.js +++ b/frontend/src/metabase/entities/index.js @@ -1,4 +1,5 @@ export collections from "./collections"; +export snippetCollections from "./snippet-collections"; export dashboards from "./dashboards"; export pulses from "./pulses"; export questions from "./questions"; diff --git a/frontend/src/metabase/entities/search.js b/frontend/src/metabase/entities/search.js index 345ad56a392ffe602bf5c8b5098c79fe883e1a7c..45ed701f969f94f90f89f46e478d7e3a8735c3d2 100644 --- a/frontend/src/metabase/entities/search.js +++ b/frontend/src/metabase/entities/search.js @@ -22,20 +22,29 @@ export default createEntity({ api: { list: async (query = {}) => { if (query.collection) { - const { collection, archived, model, ...unsupported } = query; + const { + collection, + archived, + model, + namespace, + ...unsupported + } = query; if (Object.keys(unsupported).length > 0) { throw new Error( "search with `collection` filter does not support these filters: " + Object.keys(unsupported).join(", "), ); } - return (await collectionList({ collection, archived, model })).map( - item => ({ - collection_id: canonicalCollectionId(collection), - archived: archived || false, - ...item, - }), - ); + return (await collectionList({ + collection, + archived, + model, + namespace, + })).map(item => ({ + collection_id: canonicalCollectionId(collection), + archived: archived || false, + ...item, + })); } else { return searchList(query); } diff --git a/frontend/src/metabase/entities/snippet-collections.js b/frontend/src/metabase/entities/snippet-collections.js new file mode 100644 index 0000000000000000000000000000000000000000..4034a747af4b508847583b185ce1c4b63e0b3add --- /dev/null +++ b/frontend/src/metabase/entities/snippet-collections.js @@ -0,0 +1,106 @@ +/* @flow */ +import _ from "underscore"; +import { t } from "ttag"; +import { createSelector } from "reselect"; + +import { color } from "metabase/lib/colors"; +import { createEntity, undo } from "metabase/lib/entities"; +import { SnippetCollectionSchema } from "metabase/schema"; +import NormalCollections, { + canonicalCollectionId, + getExpandedCollectionsById, +} from "metabase/entities/collections"; + +const SnippetCollections = createEntity({ + name: "snippetCollections", + schema: SnippetCollectionSchema, + + api: _.mapObject(NormalCollections.api, f => (first, ...rest) => + f({ ...first, namespace: "snippets" }, ...rest), + ), + + displayNameOne: t`snippet collection`, + displayNameMany: t`snippet collections`, + + objectActions: { + setArchived: ({ id }, archived, opts) => + SnippetCollections.actions.update( + { id }, + { archived }, + undo(opts, "folder", archived ? "archived" : "unarchived"), + ), + + setCollection: ({ id }, collection, opts) => + SnippetCollections.actions.update( + { id }, + { parent_id: canonicalCollectionId(collection && collection.id) }, + undo(opts, "folder", "moved"), + ), + + // NOTE: DELETE not currently implemented + // $FlowFixMe: no official way to disable builtin actions yet + delete: null, + }, + + selectors: { + getExpandedCollectionsById: createSelector( + [ + state => state.entities.snippetCollections, + state => state.entities.snippetCollections_list[null] || [], + ], + (collections, collectionsIds) => + getExpandedCollectionsById( + collectionsIds.map(id => collections[id]), + null, + ), + ), + }, + + createSelectors: ({ getObject, getFetched }) => ({ + getFetched: (state, props) => + getFetched(state, props) || getObject(state, props), + }), + + objectSelectors: { + getIcon: collection => "folder", + }, + + form: { + fields: [ + { + name: "name", + title: t`Give your folder a name`, + placeholder: t`Something short but sweet`, + validate: name => + (!name && t`Name is required`) || + (name && name.length > 100 && t`Name must be 100 characters or less`), + }, + { + name: "description", + title: t`Add a description`, + type: "text", + placeholder: t`It's optional but oh, so helpful`, + normalize: description => description || null, // expected to be nil or non-empty string + }, + { + name: "color", + title: t`Color`, + type: "hidden", + initial: () => color("brand"), + validate: color => !color && t`Color is required`, + }, + { + name: "parent_id", + title: t`Folder this should be in`, + type: "snippetCollection", + normalize: canonicalCollectionId, + }, + ], + }, + + getAnalyticsMetadata([object], { action }, getState) { + return undefined; // TODO: is there anything informative to track here? + }, +}); + +export default SnippetCollections; diff --git a/frontend/src/metabase/entities/snippets.js b/frontend/src/metabase/entities/snippets.js index 45f9091413e8c4fd21d2a44f91a3c652508c5d90..ec96021dd9abf0d610d86e6f10c33f9990b3954e 100644 --- a/frontend/src/metabase/entities/snippets.js +++ b/frontend/src/metabase/entities/snippets.js @@ -2,35 +2,63 @@ import { t } from "ttag"; import { createEntity } from "metabase/lib/entities"; import validate from "metabase/lib/validate"; +import { canonicalCollectionId } from "metabase/entities/collections"; -export default createEntity({ +const formFields = [ + { + name: "content", + title: t`Enter some SQL here so you can reuse it later`, + placholder: "AND canceled_at IS null\nAND account_type = 'PAID'", + type: "text", + className: + "Form-input full text-monospace text-normal text-small bg-light text-spaced", + rows: 4, + validate: validate.required().maxLength(10000), + }, + { + name: "name", + title: t`Give your snippet a name`, + placeholder: t`Current Customers`, + validate: validate.required().maxLength(100), + }, + { + name: "description", + title: t`Add a description`, + placeholder: t`It's optional but oh, so helpful`, + validate: validate.maxLength(500), + }, +]; + +const Snippets = createEntity({ name: "snippets", nameOne: "snippet", path: "/api/native-query-snippet", - form: { - fields: [ - { - name: "content", - title: t`Enter some SQL here so you can reuse it later`, - placeholder: "AND canceled_at IS null\nAND account_type = 'PAID'", - type: "text", - className: - "Form-input full text-monospace text-normal text-small bg-light text-spaced", - rows: 4, - validate: validate.required().maxLength(10000), - }, - { - name: "name", - title: t`Give your snippet a name`, - placeholder: t`Current Customers`, - validate: validate.required().maxLength(100), - }, - { - name: "description", - title: t`Add a description`, - placeholder: t`It's optional but oh, so helpful`, - validate: validate.maxLength(500), - }, - ], + createSelectors: ({ getObject, getFetched }) => ({ + getFetched: (state, props) => + getFetched(state, props) || getObject(state, props), + }), + forms: { + withoutVisibleCollectionPicker: { + fields: [ + ...formFields, + { + name: "collection_id", + hidden: true, + }, + ], + }, + withVisibleCollectionPicker: { + fields: [ + ...formFields, + { + name: "collection_id", + title: t`Folder this should be in`, + type: "snippetCollection", + normalize: canonicalCollectionId, + }, + ], + }, }, }); + +export default Snippets; diff --git a/frontend/src/metabase/lib/entities.js b/frontend/src/metabase/lib/entities.js index 8f719742c0c21dc60c7ecd3a0912848e50dfa6e3..c87114c596096f34a77285fe01a2551e883753da 100644 --- a/frontend/src/metabase/lib/entities.js +++ b/frontend/src/metabase/lib/entities.js @@ -66,6 +66,9 @@ type EntityDefinition = { selectors?: { [name: string]: Function, }, + createSelectors?: ({ [name: string]: Function }) => { + [name: string]: Function, + }, objectActions?: { [name: string]: ObjectActionCreator, }, @@ -485,9 +488,9 @@ export function createEntity(def: EntityDefinition): Entity { // delegate to getObject (state, entityIds) => entityIds && - entityIds.map(entityId => - entity.selectors.getObject(state, { entityId }), - ), + entityIds + .map(entityId => entity.selectors.getObject(state, { entityId })) + .filter(e => e != null), // deleted entities might remain in lists ); // REQUEST STATE SELECTORS @@ -527,14 +530,18 @@ export function createEntity(def: EntityDefinition): Entity { requestState => requestState.error, ); - entity.selectors = { + const defaultSelectors = { getList, getObject, getFetched, getLoading, getLoaded, getError, + }; + entity.selectors = { + ...defaultSelectors, ...(def.selectors || {}), + ...(def.createSelectors ? def.createSelectors(defaultSelectors) : {}), }; entity.objectSelectors = { diff --git a/frontend/src/metabase/lib/groups.js b/frontend/src/metabase/lib/groups.js index eab752b9548c24b40a0413afde89212432cc73ae..585628a1f4901219effba0b22abc0477b39a1d36 100644 --- a/frontend/src/metabase/lib/groups.js +++ b/frontend/src/metabase/lib/groups.js @@ -19,7 +19,7 @@ export function isMetaBotGroup(group) { } export function isSpecialGroup(group) { - return isDefaultGroup(group) || isAdminGroup(group) || isMetaBotGroup(group); + return isDefaultGroup(group) || isMetaBotGroup(group); } export function canEditPermissions(group) { diff --git a/frontend/src/metabase/lib/query/description.js b/frontend/src/metabase/lib/query/description.js index f308ddf008f691efb953f680ea59ddf58c33ecd6..00b2748dcc51f11dd48f9adb917b01c2f0cc5d4f 100644 --- a/frontend/src/metabase/lib/query/description.js +++ b/frontend/src/metabase/lib/query/description.js @@ -345,7 +345,6 @@ export function formatQueryDescription(parts, options = {}) { if (!parts) { return ""; } - console.log("formatQueryDescription", { parts, options }); options = { jsx: false, @@ -370,7 +369,7 @@ export function formatQueryDescription(parts, options = {}) { }; // these array gymnastics are needed to support JSX formatting - const sections = options.sections + const sections = (options.sections || []) .map(section => _.flatten(sectionFns[section](parts, options)).filter(s => !!s), ) diff --git a/frontend/src/metabase/lib/redux.js b/frontend/src/metabase/lib/redux.js index 19a2dd5e3dcdd0fcf0a07b270c08724aa5e569d3..e8313b2e0dc1796310e30a32c3513c1107dbe646 100644 --- a/frontend/src/metabase/lib/redux.js +++ b/frontend/src/metabase/lib/redux.js @@ -134,10 +134,10 @@ export const updateData = async ({ export function mergeEntities(entities, newEntities) { entities = { ...entities }; for (const id in newEntities) { - if (id in entities) { - entities[id] = { ...entities[id], ...newEntities[id] }; + if (newEntities[id] === null) { + delete entities[id]; } else { - entities[id] = newEntities[id]; + entities[id] = { ...entities[id], ...newEntities[id] }; } } return entities; diff --git a/frontend/src/metabase/lib/settings.js b/frontend/src/metabase/lib/settings.js index 8f0165feb5d6aa802dac6f25dde1cb99ed902976..16ebdcf37b63c8b43a1a047a167a729ab7b80fb0 100644 --- a/frontend/src/metabase/lib/settings.js +++ b/frontend/src/metabase/lib/settings.js @@ -14,6 +14,7 @@ export type SettingName = | "custom-geojson" | "email-configured?" | "enable-embedding" + | "enable-enhancements?" | "enable-public-sharing" | "enable-xrays" | "engines" @@ -76,6 +77,10 @@ class Settings { return this.get("admin-email"); } + enhancementsEnabled() { + return this.get("enable-enhancements?"); + } + isEmailConfigured() { return this.get("email-configured?"); } diff --git a/frontend/src/metabase/modes/components/drill/SummarizeColumnByTimeDrill.js b/frontend/src/metabase/modes/components/drill/SummarizeColumnByTimeDrill.js index 4e8e2475a35ec6e50103d2d1d3abc7610b783d92..31d321739e0cb6233e24d9f6e7b7f6a93e413b20 100644 --- a/frontend/src/metabase/modes/components/drill/SummarizeColumnByTimeDrill.js +++ b/frontend/src/metabase/modes/components/drill/SummarizeColumnByTimeDrill.js @@ -50,6 +50,6 @@ export default ({ ? [aggregator.short, fieldRefForColumn(column)] : [aggregator.short], ) - .pivot([dateDimension.defaultDimension().mbql()]), + .pivot([dateDimension.defaultBreakout()]), })); }; diff --git a/frontend/src/metabase/parameters/components/Parameters.jsx b/frontend/src/metabase/parameters/components/Parameters.jsx index ade7ec814f111a7a0c2a48690767b4166b27c937..c31b35893d5328c0dbba5a324d3feb38e5116d66 100644 --- a/frontend/src/metabase/parameters/components/Parameters.jsx +++ b/frontend/src/metabase/parameters/components/Parameters.jsx @@ -268,7 +268,8 @@ export function parseQueryParam( } // [].every is always true, so only check if there are some fields if (fields.length > 0) { - if (fields.every(f => f.isNumeric())) { + // unix dates fields are numeric but query params shouldn't be parsed as numbers + if (fields.every(f => f.isNumeric() && !f.isDate())) { return parseFloat(value); } if (fields.every(f => f.isBoolean())) { diff --git a/frontend/src/metabase/plugins/index.js b/frontend/src/metabase/plugins/index.js index 8cdbd901e79a633f7511c1cde6a7837542c9a843..55d9cee763a5437730663fb8d2752187f0abf7a4 100644 --- a/frontend/src/metabase/plugins/index.js +++ b/frontend/src/metabase/plugins/index.js @@ -60,3 +60,9 @@ export const PLUGIN_SELECTORS = { getShowAuthScene: (state, props) => true, getLogoBackgroundClass: (state, props) => "bg-white", }; + +// snippet sidebar +export const PLUGIN_SNIPPET_SIDEBAR_PLUS_MENU_OPTIONS = []; +export const PLUGIN_SNIPPET_SIDEBAR_ROW_RENDERERS = {}; +export const PLUGIN_SNIPPET_SIDEBAR_MODALS = []; +export const PLUGIN_SNIPPET_SIDEBAR_HEADER_BUTTONS = []; diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js index 8b836ffcd4dbfda7b648957c51bad4cdb0a2fd40..666eddf3d7b7c13df6bb5db7c007150977ac35df 100644 --- a/frontend/src/metabase/query_builder/actions.js +++ b/frontend/src/metabase/query_builder/actions.js @@ -48,6 +48,7 @@ import { getIsRunning, getNativeEditorCursorOffset, getNativeEditorSelectedText, + getSnippetCollectionId, } from "./selectors"; import { MetabaseApi, CardApi, UserApi } from "metabase/services"; @@ -569,9 +570,15 @@ export const setNativeEditorSelectedRange = createAction( export const SET_MODAL_SNIPPET = "metabase/qb/SET_MODAL_SNIPPET"; export const setModalSnippet = createAction(SET_MODAL_SNIPPET); +export const SET_SNIPPET_COLLECTION_ID = + "metabase/qb/SET_SNIPPET_COLLECTION_ID"; +export const setSnippetCollectionId = createAction(SET_SNIPPET_COLLECTION_ID); + export const openSnippetModalWithSelectedText = () => (dispatch, getState) => { - const content = getNativeEditorSelectedText(getState()); - dispatch(setModalSnippet({ content })); + const state = getState(); + const content = getNativeEditorSelectedText(state); + const collection_id = getSnippetCollectionId(state); + dispatch(setModalSnippet({ content, collection_id })); }; export const closeSnippetModal = () => (dispatch, getState) => { diff --git a/frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx b/frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx index 3e1ea1b69995f36f9c1beec7941a78360f91e70d..eca78d5a1c3d9055ba128c6560956a0f41f6a262 100644 --- a/frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx +++ b/frontend/src/metabase/query_builder/components/NativeQueryEditor.jsx @@ -39,6 +39,7 @@ import ExplicitSize from "metabase/components/ExplicitSize"; import Popover from "metabase/components/Popover"; import Snippets from "metabase/entities/snippets"; +import SnippetCollections from "metabase/entities/snippet-collections"; import Parameters from "metabase/parameters/components/Parameters"; @@ -111,6 +112,7 @@ type Props = { insertSnippet: () => void, closeSnippetModal: () => void, snippets: { name: string }[], + snippetCollections: { can_write: boolean }[], }; type State = { initialHeight: number, @@ -119,6 +121,7 @@ type State = { @ExplicitSize() @Snippets.loadList({ loadingAndErrorWrapper: false }) +@SnippetCollections.loadList({ loadingAndErrorWrapper: false }) export default class NativeQueryEditor extends Component { props: Props; state: State; @@ -481,12 +484,23 @@ export default class NativeQueryEditor extends Component { isRunning, isResultDirty, isPreviewing, + snippetCollections, + snippets, } = this.props; const database = query.database(); const databases = query.metadata().databasesList({ savedQuestions: false }); const parameters = query.question().parameters(); + // hide the snippet sidebar if there aren't any visible snippets/collections and the root collection isn't writable + const showSnippetSidebarButton = !( + snippets && + snippets.length === 0 && + snippetCollections && + snippetCollections.length === 1 && + snippetCollections[0].can_write === false + ); + let dataSelectors = []; if (isNativeEditorOpen && databases.length > 0) { // we only render a db selector if there are actually multiple to choose from @@ -648,11 +662,13 @@ export default class NativeQueryEditor extends Component { size={ICON_SIZE} className="mt3" /> - <SnippetSidebarButton - {...this.props} - size={ICON_SIZE} - className="mt3" - /> + {showSnippetSidebarButton && ( + <SnippetSidebarButton + {...this.props} + size={ICON_SIZE} + className="mt3" + /> + )} <RunButtonWithTooltip disabled={!isRunnable} isRunning={isRunning} diff --git a/frontend/src/metabase/query_builder/components/template_tags/SnippetModal.jsx b/frontend/src/metabase/query_builder/components/template_tags/SnippetModal.jsx index ef9476c730d494709e05b1f3b4ff6bf2d3084cba..c2aceafba1279bc03cee50c50b9651f33a79eb15 100644 --- a/frontend/src/metabase/query_builder/components/template_tags/SnippetModal.jsx +++ b/frontend/src/metabase/query_builder/components/template_tags/SnippetModal.jsx @@ -2,18 +2,32 @@ import React from "react"; import { t } from "ttag"; -import colors from "metabase/lib/colors"; +import Icon from "metabase/components/Icon"; import Modal from "metabase/components/Modal"; import Link from "metabase/components/Link"; import Snippets from "metabase/entities/snippets"; +import SnippetCollections from "metabase/entities/snippet-collections"; +@SnippetCollections.loadList() export default class SnippetModal extends React.Component { render() { - const { insertSnippet, onSnippetUpdate, closeModal, snippet } = this.props; + const { + insertSnippet, + onSnippetUpdate, + closeModal, + snippet, + snippetCollections, + } = this.props; + return ( <Modal onClose={closeModal}> <Snippets.ModalForm snippet={snippet} + form={ + snippetCollections.length <= 1 + ? Snippets.forms.withoutVisibleCollectionPicker + : Snippets.forms.withVisibleCollectionPicker + } title={ snippet.id != null ? t`Editing ${snippet.name}` @@ -29,18 +43,20 @@ export default class SnippetModal extends React.Component { closeModal(); }} onClose={closeModal} // the "x" button - onCance={closeModal} // the cancel button submitTitle={t`Save`} footerExtraButtons={ // only display archive for saved snippets snippet.id != null ? ( <Link - style={{ color: colors.error }} onClick={async () => { await snippet.update({ archived: true }); closeModal(); }} - >{t`Archive`}</Link> + className="flex align-center text-medium text-bold" + > + <Icon name="archive" className="mr1" /> + {t`Archive`} + </Link> ) : null } /> diff --git a/frontend/src/metabase/query_builder/components/template_tags/SnippetSidebar.jsx b/frontend/src/metabase/query_builder/components/template_tags/SnippetSidebar.jsx index c9d3fb24e06df53ceaab59f3eb0fa62d293c13fa..6d97f708fb4dd5ea084818c605f457de891375eb 100644 --- a/frontend/src/metabase/query_builder/components/template_tags/SnippetSidebar.jsx +++ b/frontend/src/metabase/query_builder/components/template_tags/SnippetSidebar.jsx @@ -2,17 +2,28 @@ import React from "react"; import PropTypes from "prop-types"; - +import { connect } from "react-redux"; import { t } from "ttag"; import cx from "classnames"; +import _ from "underscore"; +import { + PLUGIN_SNIPPET_SIDEBAR_PLUS_MENU_OPTIONS, + PLUGIN_SNIPPET_SIDEBAR_ROW_RENDERERS, + PLUGIN_SNIPPET_SIDEBAR_MODALS, + PLUGIN_SNIPPET_SIDEBAR_HEADER_BUTTONS, +} from "metabase/plugins"; import Icon from "metabase/components/Icon"; -import Button from "metabase/components/Button"; +import PopoverWithTrigger from "metabase/components/PopoverWithTrigger"; import SidebarContent from "metabase/query_builder/components/SidebarContent"; import SidebarHeader from "metabase/query_builder/components/SidebarHeader"; +import SnippetRow from "./snippet-sidebar/SnippetRow"; import { color } from "metabase/lib/colors"; import Snippets from "metabase/entities/snippets"; +import SnippetCollections from "metabase/entities/snippet-collections"; +import { canonicalCollectionId } from "metabase/entities/collections"; +import Search from "metabase/entities/search"; import type { Snippet } from "metabase-types/types/Snippet"; @@ -22,6 +33,10 @@ type Props = { openSnippetModalWithSelectedText: () => void, insertSnippet: () => void, snippets: Snippet[], + snippetCollection: any, + snippetCollections: any[], + search: any[], + setSnippetCollectionId: () => void, }; type State = { @@ -34,7 +49,20 @@ const ICON_SIZE = 16; const HEADER_ICON_SIZE = 18; const MIN_SNIPPETS_FOR_SEARCH = 15; -@Snippets.loadList({ wrapped: true }) +@Snippets.loadList() +@SnippetCollections.loadList() +@SnippetCollections.load({ + id: (state, props) => + props.snippetCollectionId === null ? "root" : props.snippetCollectionId, + wrapped: true, +}) +@Search.loadList({ + query: (state, props) => ({ + collection: + props.snippetCollectionId === null ? "root" : props.snippetCollectionId, + namespace: "snippets", + }), +}) export default class SnippetSidebar extends React.Component { props: Props; state: State = { @@ -79,13 +107,13 @@ export default class SnippetSidebar extends React.Component { ); render() { - const { snippets, openSnippetModalWithSelectedText } = this.props; + const { + snippets, + openSnippetModalWithSelectedText, + snippetCollection, + search, + } = this.props; const { showSearch, searchString, showArchived } = this.state; - const filteredSnippets = snippets.filter( - snippet => - !showSearch || - snippet.name.toLowerCase().includes(searchString.toLowerCase()), - ); if (showArchived) { return ( @@ -95,9 +123,17 @@ export default class SnippetSidebar extends React.Component { ); } + const displayedItems = showSearch + ? snippets.filter(snippet => + snippet.name.toLowerCase().includes(searchString.toLowerCase()), + ) + : _.sortBy(search, "model"); // relies on "collection" sorting before "snippet"; + return ( <SidebarContent footer={this.footer()}> - {snippets.length === 0 ? ( + {!showSearch && + displayedItems.length === 0 && + snippetCollection.id === "root" ? ( <div className="px3 flex flex-column align-center"> <svg viewBox="0 0 10 10" @@ -120,14 +156,14 @@ export default class SnippetSidebar extends React.Component { <div> <div className="flex align-center pl3 pr2" - style={{ paddingTop: 10, paddingBottom: 8 }} + style={{ paddingTop: 10, paddingBottom: 11 }} > <div className="flex-full"> <div /* Hide the search input by collapsing dimensions rather than `display: none`. This allows us to immediately focus on it when showSearch is set to true.*/ style={showSearch ? {} : { width: 0, height: 0 }} - className="overflow-hidden" + className="text-heavy h3 overflow-hidden" > <input className="input input--borderless p0" @@ -143,35 +179,99 @@ export default class SnippetSidebar extends React.Component { }} /> </div> - <span - className={cx({ hide: showSearch }, "text-heavy h3")} - >{t`Snippets`}</span> + <span className={cx({ hide: showSearch }, "text-heavy h3")}> + {snippetCollection.id === "root" ? ( + t`Snippets` + ) : ( + <span + className="text-brand-hover cursor-pointer" + onClick={() => { + const parentId = snippetCollection.parent_id; + this.props.setSnippetCollectionId( + // if this collection's parent isn't in the list, we don't have perms to see it, return to the root instead + this.props.snippetCollections.some( + sc => + canonicalCollectionId(sc.id) === + canonicalCollectionId(parentId), + ) + ? parentId + : null, + ); + }} + > + <Icon name="chevronleft" className="mr1" /> + {snippetCollection.name} + </span> + )} + </span> </div> - <div className="flex-align-right text-medium no-decoration"> + <div className="flex-align-right flex align-center text-medium no-decoration"> + {[ + ...PLUGIN_SNIPPET_SIDEBAR_HEADER_BUTTONS.map(f => + f(this, { className: "mr2" }), + ), + ]} {snippets.length >= MIN_SNIPPETS_FOR_SEARCH && ( <Icon className={cx( { hide: showSearch }, - "text-brand-hover cursor-pointer mr2", + "text-brand-hover cursor-pointer mr1", )} onClick={this.showSearch} name="search" size={HEADER_ICON_SIZE} /> )} - <Icon - className={cx( - { hide: showSearch }, - "text-brand bg-light-hover rounded p1 cursor-pointer", - )} - onClick={openSnippetModalWithSelectedText} - name="add" - size={HEADER_ICON_SIZE} - /> + + {snippetCollection.can_write && ( + <PopoverWithTrigger + triggerClasses="flex" + triggerElement={ + <Icon + className={cx( + { hide: showSearch }, + "text-brand bg-light-hover rounded p1 cursor-pointer", + )} + name="add" + size={HEADER_ICON_SIZE} + /> + } + > + {({ onClose }) => ( + <div className="flex flex-column"> + {[ + { + icon: "snippet", + name: t`New snippet`, + onClick: openSnippetModalWithSelectedText, + }, + ...PLUGIN_SNIPPET_SIDEBAR_PLUS_MENU_OPTIONS.map(f => + f(this), + ), + ].map(({ icon, name, onClick }) => ( + <div + className="p2 bg-medium-hover flex cursor-pointer text-brand-hover" + onClick={() => { + onClick(); + onClose(); + }} + > + <Icon + name={icon} + size={ICON_SIZE} + className="mr2" + /> + <h4>{name}</h4> + </div> + ))} + </div> + )} + </PopoverWithTrigger> + )} <Icon className={cx( { hide: !showSearch }, - "text-brand-hover cursor-pointer", + "p1 text-brand-hover cursor-pointer", )} onClick={this.hideSearch} name="close" @@ -180,26 +280,44 @@ export default class SnippetSidebar extends React.Component { </div> </div> <div className="flex-full"> - {filteredSnippets.map(snippet => ( - <SnippetRow - key={snippet.id} - snippet={snippet} - insertSnippet={this.props.insertSnippet} - setModalSnippet={this.props.setModalSnippet} - /> - ))} + {displayedItems.length > 0 + ? displayedItems.map(item => ( + <Row + key={`${item.model || "snippet"}-${item.id}`} + item={item} + type={item.model || "snippet"} + setSidebarState={this.setState.bind(this)} + canWrite={snippetCollection.can_write} + {...this.props} + /> + )) + : null} </div> </div> )} + {PLUGIN_SNIPPET_SIDEBAR_MODALS.map(f => f(this))} </SidebarContent> ); } } +@SnippetCollections.loadList({ query: { archived: true }, wrapped: true }) +@connect((state, { list }) => ({ archivedSnippetCollections: list })) +@SnippetCollections.loadList() @Snippets.loadList({ query: { archived: true }, wrapped: true }) class ArchivedSnippets extends React.Component { render() { - const { onBack, snippets } = this.props; + const { + onBack, + snippets, + snippetCollections, + archivedSnippetCollections, + } = this.props; + const collectionsById = _.indexBy( + snippetCollections.concat(archivedSnippetCollections), + c => canonicalCollectionId(c.id), + ); + return ( <SidebarContent> <SidebarHeader @@ -208,99 +326,35 @@ class ArchivedSnippets extends React.Component { onBack={onBack} /> + {archivedSnippetCollections.map(collection => ( + <Row + key={`collection-${collection.id}`} + item={collection} + type="collection" + /> + ))} {snippets.map(snippet => ( - <SnippetRow - key={snippet.id} - snippet={snippet} - unarchiveSnippet={() => snippet.update({ archived: false })} + <Row + key={`snippet-${snippet.id}`} + item={snippet} + type="snippet" + canWrite={ + collectionsById[ + // `String` used to appease flow + String(canonicalCollectionId(snippet.collection_id)) + ].can_write + } /> ))} </SidebarContent> ); } } -class SnippetRow extends React.Component { - state: { isOpen: boolean }; - - constructor(props) { - super(props); - this.state = { isOpen: false }; - } - render() { - const { - snippet, - insertSnippet, - setModalSnippet, - unarchiveSnippet, - } = this.props; - const { description, content } = snippet; - const { isOpen } = this.state; - return ( - <div - className={cx( - { "border-transparent": !isOpen }, - "border-bottom border-top", - )} - > - <div - className="cursor-pointer bg-light-hover text-bold flex align-center justify-between py2 px3 hover-parent hover--display" - onClick={() => this.setState({ isOpen: !isOpen })} - > - <div - className="flex text-brand-hover" - onClick={ - unarchiveSnippet - ? () => this.setState({ isOpen: true }) - : e => { - e.stopPropagation(); - insertSnippet(snippet); - } - } - > - <Icon - name="snippet" - size={ICON_SIZE} - className="hover-child--hidden text-light" - /> - <Icon - name={insertSnippet ? "arrow_left_to_line" : "snippet"} - size={ICON_SIZE} - className="hover-child" - /> - <span className="flex-full ml1">{snippet.name}</span> - </div> - <Icon - name={isOpen ? "chevronup" : "chevrondown"} - size={ICON_SIZE} - className={cx({ "hover-child": !isOpen })} - /> - </div> - {isOpen && ( - <div className="px3 pb2 pt1"> - {description && <p className="text-medium mt0">{description}</p>} - <pre - className="bg-light bordered rounded p1 text-monospace text-small text-pre-wrap overflow-scroll overflow-x-scroll" - style={{ maxHeight: 320 }} - > - {content} - </pre> - <Button - onClick={ - unarchiveSnippet - ? unarchiveSnippet - : () => setModalSnippet(snippet) - } - borderless - medium - className="text-brand text-white-hover bg-light bg-brand-hover mt1" - icon={unarchiveSnippet ? "unarchive" : "pencil"} - > - {unarchiveSnippet ? t`Unarchive` : t`Edit`} - </Button> - </div> - )} - </div> - ); - } +function Row(props) { + const Component = { + snippet: SnippetRow, + ...PLUGIN_SNIPPET_SIDEBAR_ROW_RENDERERS, + }[props.type]; + return Component ? <Component {...props} /> : null; } diff --git a/frontend/src/metabase/query_builder/components/template_tags/snippet-sidebar/SnippetRow.jsx b/frontend/src/metabase/query_builder/components/template_tags/snippet-sidebar/SnippetRow.jsx new file mode 100644 index 0000000000000000000000000000000000000000..5c1e9d06c8b6e1e0374a2245037d33af16930c77 --- /dev/null +++ b/frontend/src/metabase/query_builder/components/template_tags/snippet-sidebar/SnippetRow.jsx @@ -0,0 +1,99 @@ +import React from "react"; +import { t } from "ttag"; + +import cx from "classnames"; +import Icon from "metabase/components/Icon"; +import Button from "metabase/components/Button"; +import Snippets from "metabase/entities/snippets"; + +const ICON_SIZE = 16; + +@Snippets.load({ + id: (state, props) => props.item.id, + wrapped: true, +}) +class SnippetRow extends React.Component { + state: { isOpen: boolean }; + + constructor(props) { + super(props); + this.state = { isOpen: false }; + } + + render() { + const { snippet, insertSnippet, setModalSnippet, canWrite } = this.props; + + const { description, content } = snippet; + const { isOpen } = this.state; + return ( + <div + className={cx( + { "border-transparent": !isOpen }, + "border-bottom border-top", + )} + > + <div + className="cursor-pointer bg-light-hover text-bold flex align-center justify-between py2 px3 hover-parent hover--display" + onClick={() => this.setState({ isOpen: !isOpen })} + > + <div + className="flex text-brand-hover" + onClick={ + snippet.archived + ? () => this.setState({ isOpen: true }) + : e => { + e.stopPropagation(); + insertSnippet(snippet); + } + } + > + <Icon + name="snippet" + size={ICON_SIZE} + className="hover-child--hidden text-light" + /> + <Icon + name={insertSnippet ? "arrow_left_to_line" : "snippet"} + size={ICON_SIZE} + className="hover-child" + /> + <span className="flex-full ml1">{snippet.name}</span> + </div> + <Icon + name={isOpen ? "chevronup" : "chevrondown"} + size={ICON_SIZE} + className={cx({ "hover-child": !isOpen })} + /> + </div> + {isOpen && ( + <div className="px3 pb2 pt1"> + {description && <p className="text-medium mt0">{description}</p>} + <pre + className="bg-light bordered rounded p1 text-monospace text-small text-pre-wrap overflow-scroll overflow-x-scroll" + style={{ maxHeight: 320 }} + > + {content} + </pre> + {canWrite && ( + <Button + onClick={ + snippet.archived + ? () => snippet.update({ archived: false }) + : () => setModalSnippet(snippet) + } + borderless + medium + className="text-brand text-white-hover bg-light bg-brand-hover mt1" + icon={snippet.archived ? "unarchive" : "pencil"} + > + {snippet.archived ? t`Unarchive` : t`Edit`} + </Button> + )} + </div> + )} + </div> + ); + } +} + +export default SnippetRow; diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx index d7964205fa7f9edbbbdb3f9f1c701a310492cdac..a0b477299fecfa0886a902af52dfa4fc8335a2a1 100644 --- a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx +++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx @@ -38,6 +38,7 @@ import { getIsResultDirty, getMode, getModalSnippet, + getSnippetCollectionId, getQuery, getQuestion, getOriginalQuestion, @@ -146,6 +147,7 @@ const mapStateToProps = (state, props) => { nativeEditorCursorOffset: getNativeEditorCursorOffset(state), nativeEditorSelectedText: getNativeEditorSelectedText(state), modalSnippet: getModalSnippet(state), + snippetCollectionId: getSnippetCollectionId(state), }; }; diff --git a/frontend/src/metabase/query_builder/reducers.js b/frontend/src/metabase/query_builder/reducers.js index 4ce5962b27e2179895b2eef307a8b98c635e9582..6154847b7bc73f589ffe3f52e258af1d73fb32ed 100644 --- a/frontend/src/metabase/query_builder/reducers.js +++ b/frontend/src/metabase/query_builder/reducers.js @@ -11,6 +11,7 @@ import { SET_IS_SHOWING_TEMPLATE_TAGS_EDITOR, SET_NATIVE_EDITOR_SELECTED_RANGE, SET_MODAL_SNIPPET, + SET_SNIPPET_COLLECTION_ID, CLOSE_QB_NEWB_MODAL, RELOAD_CARD, API_CREATE_QUESTION, @@ -59,6 +60,7 @@ const DEFAULT_UI_CONTROLS = { isPreviewing: true, // sql preview mode isShowingRawTable: false, // table/viz toggle queryBuilderMode: false, // "view" or "notebook" + snippetCollectionId: null, }; const UI_CONTROLS_SIDEBAR_DEFAULTS = { @@ -114,6 +116,7 @@ export const uiControls = handleActions( ...state, ...CLOSED_NATIVE_EDITOR_SIDEBARS, isShowingSnippetSidebar: !state.isShowingSnippetSidebar, + snippetCollectionId: null, }), }, [SET_IS_SHOWING_TEMPLATE_TAGS_EDITOR]: { @@ -131,6 +134,10 @@ export const uiControls = handleActions( ...state, modalSnippet: payload, }), + [SET_SNIPPET_COLLECTION_ID]: (state, { payload }) => ({ + ...state, + snippetCollectionId: payload, + }), [CLOSE_QB_NEWB_MODAL]: { next: (state, { payload }) => ({ ...state, isShowingNewbModal: false }), }, diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js index c6e4c6897c9e945105a83e74be64ca374283c06d..c607c7634789c7dc57dd38240689753c3e77192d 100644 --- a/frontend/src/metabase/query_builder/selectors.js +++ b/frontend/src/metabase/query_builder/selectors.js @@ -393,6 +393,11 @@ export const getModalSnippet = createSelector( uiControls => uiControls && uiControls.modalSnippet, ); +export const getSnippetCollectionId = createSelector( + [getUiControls], + uiControls => uiControls && uiControls.snippetCollectionId, +); + /** * Returns whether the query can be "preview", i.e. native query editor is open and visualization is table * NOTE: completely disabled for now diff --git a/frontend/src/metabase/schema.js b/frontend/src/metabase/schema.js index b7c1fe5802425aac5f8fabb0b359167641d49cfd..833c15df9c77b2cf0b1e3161de1dffa73bdb03f6 100644 --- a/frontend/src/metabase/schema.js +++ b/frontend/src/metabase/schema.js @@ -32,6 +32,8 @@ export const TableSchema = new schema.Entity( export const FieldSchema = new schema.Entity("fields"); export const SegmentSchema = new schema.Entity("segments"); export const MetricSchema = new schema.Entity("metrics"); +export const SnippetSchema = new schema.Entity("snippets"); +export const SnippetCollectionSchema = new schema.Entity("snippetCollections"); DatabaseSchema.define({ tables: [TableSchema], @@ -82,6 +84,8 @@ export const ENTITIES_SCHEMA_MAP = { collections: CollectionSchema, segments: SegmentSchema, metrics: MetricSchema, + snippets: SnippetSchema, + snippetCollections: SnippetCollectionSchema, }; export const ObjectUnionSchema = new schema.Union( diff --git a/frontend/src/metabase/visualizations/lib/utils.js b/frontend/src/metabase/visualizations/lib/utils.js index 371dd47a69c070143b150b9e5e771f1ece09e37e..537cb868f8f6f11d823d9d981c406d9f3ea88c54 100644 --- a/frontend/src/metabase/visualizations/lib/utils.js +++ b/frontend/src/metabase/visualizations/lib/utils.js @@ -234,6 +234,7 @@ export function getCardAfterVisualizationClick(nextCard, previousCard) { isMultiseriesQuestion ? previousCard.id : nextCard.id, + id: null, }; } else { // Even though the card is currently clean, we might still apply dashboard parameters to it, diff --git a/frontend/test/__runner__/backend.js b/frontend/test/__runner__/backend.js index 09b4c35760fb3b8a1800cb0b611b5b4ccb326add..03b52d329cb3e2affe87639730a7a255399966f0 100644 --- a/frontend/test/__runner__/backend.js +++ b/frontend/test/__runner__/backend.js @@ -64,6 +64,10 @@ export const BackendResource = createSharedResource("BackendResource", { MB_JETTY_HOST: "0.0.0.0", MB_JETTY_PORT: server.port, MB_ENABLE_TEST_ENDPOINTS: "true", + MB_PREMIUM_EMBEDDING_TOKEN: + (process.env["ENABLE_ENTERPRISE_EDITION"] === "true" && + process.env["ENTERPRISE_TOKEN"]) || + undefined, }, stdio: process.env["DISABLE_LOGGING"] || diff --git a/frontend/test/__support__/cypress.js b/frontend/test/__support__/cypress.js index 66994ba7b13f9345b57c15c47f856d916ce0c57a..09e32597a9eb8f53ec7c8c24529f7616146bb58e 100644 --- a/frontend/test/__support__/cypress.js +++ b/frontend/test/__support__/cypress.js @@ -109,23 +109,40 @@ export function typeAndBlurUsingLabel(label, value) { Cypress.on("uncaught:exception", (err, runnable) => false); -export function withSampleDataset(f) { - cy.request("GET", "/api/database/1/metadata").then(({ body }) => { - const SAMPLE_DATASET = {}; +export function withDatabase(databaseId, f) { + cy.request("GET", `/api/database/${databaseId}/metadata`).then(({ body }) => { + const database = {}; for (const table of body.tables) { const fields = {}; for (const field of table.fields) { fields[field.name] = field.id; } - SAMPLE_DATASET[table.name] = fields; - SAMPLE_DATASET[table.name + "_ID"] = table.id; + database[table.name] = fields; + database[table.name + "_ID"] = table.id; } - f(SAMPLE_DATASET); + f(database); }); } +export function withSampleDataset(f) { + return withDatabase(1, f); +} + export function visitAlias(alias) { cy.get(alias).then(url => { cy.visit(url); }); } + +export function createNativeQuestion(name, query) { + return cy.request("POST", "/api/card", { + name, + dataset_query: { + type: "native", + native: { query }, + database: 1, + }, + display: "table", + visualization_settings: {}, + }); +} diff --git a/frontend/test/metabase-smoketest/admin.cy.spec.js b/frontend/test/metabase-smoketest/admin.cy.spec.js index 4b3bd1e67119913158ea3ee5e88d06c35c2b1d92..a621973e4a65dd7b0843e3b2aee7dd48d93b8ffc 100644 --- a/frontend/test/metabase-smoketest/admin.cy.spec.js +++ b/frontend/test/metabase-smoketest/admin.cy.spec.js @@ -108,11 +108,7 @@ describe("metabase-smoketest > admin", () => { cy.findAllByText("Created At") .last() .click(); - cy.get("input[type='text']") - .wait(1) - .clear() - .wait(1) - .type("5"); + cy.get("input[type='text']").type("{selectall}{del}5"); cy.findByText("Days").click(); cy.findByText("Years").click(); sidebar() diff --git a/frontend/test/metabase/admin/people/people.e2e.spec.js b/frontend/test/metabase/admin/people/people.e2e.spec.js deleted file mode 100644 index 2aace80076253067b9526d4fffbfbc91b6822f8c..0000000000000000000000000000000000000000 --- a/frontend/test/metabase/admin/people/people.e2e.spec.js +++ /dev/null @@ -1,177 +0,0 @@ -// Converted from a Selenium E2E test -import { - createTestStore, - useSharedAdminLogin, - BROWSER_HISTORY_PUSH, - BROWSER_HISTORY_POP, -} from "__support__/e2e"; -import { - click, - clickButton, - setInputValue, - findButtonByText, -} from "__support__/enzyme"; -import { mount } from "enzyme"; -import { - CREATE_MEMBERSHIP, - LOAD_MEMBERSHIPS, -} from "metabase/admin/people/people"; -import ModalContent from "metabase/components/ModalContent"; -import { delay } from "metabase/lib/promise"; -import { getUsersWithMemberships } from "metabase/admin/people/selectors"; -import UserGroupSelect from "metabase/admin/people/components/UserGroupSelect"; -import { GroupOption } from "metabase/admin/people/components/GroupSelect"; -import { UserApi } from "metabase/services"; - -import User, { PASSWORD_RESET_MANUAL } from "metabase/entities/users"; -import Group from "metabase/entities/groups"; - -import EntityMenuTrigger from "metabase/components/EntityMenuTrigger"; -import EntityMenuItem from "metabase/components/EntityMenuItem"; - -describe("admin/people", () => { - let createdUserId = null; - - beforeAll(async () => { - useSharedAdminLogin(); - }); - - describe("user management", () => { - it("should allow admin to create new users", async () => { - const store = await createTestStore(); - store.pushPath("/admin/people"); - const app = mount(store.getAppContainer()); - - await store.waitForActions([ - User.actionTypes.FETCH_LIST, - Group.actionTypes.FETCH_LIST, - LOAD_MEMBERSHIPS, - ]); - - const email = - "testy" + Math.round(Math.random() * 10000) + "@metabase.com"; - const firstName = "Testy"; - const lastName = "McTestFace"; - - clickButton(app.find(".Button.Button--primary")); - await store.waitForActions([BROWSER_HISTORY_PUSH]); - - const addUserModal = app.find(ModalContent); - const addButton = addUserModal.find(".Button[type='submit']"); - expect(addButton.props().disabled).toBe(true); - - setInputValue(addUserModal.find("input[name='first_name']"), firstName); - setInputValue(addUserModal.find("input[name='last_name']"), lastName); - setInputValue(addUserModal.find("input[name='email']"), email); - - expect(addButton.props().disabled).toBe(false); - clickButton(addButton); - - await store.waitForActions([ - "metabase/entities/users/CREATE", - BROWSER_HISTORY_PUSH, - ]); - - await delay(100); - - // it should be a pretty safe assumption in test environment that the user that was just created has the biggest ID - const userIds = Object.keys(getUsersWithMemberships(store.getState())); - createdUserId = Math.max.apply(null, userIds.map(key => parseInt(key))); - - const userCreatedModal = app.find(ModalContent); - - click(userCreatedModal.find('[children="Show"]')); - const password = userCreatedModal.find("input").prop("value"); - - // "Done" button - click(userCreatedModal.find(".Button.Button--primary")); - - await store.waitForActions([BROWSER_HISTORY_PUSH]); - - const usersTable = app.find(".ContentTable"); - const userRow = usersTable.find(`td[children="${email}"]`).closest("tr"); - expect( - userRow - .find("td") - .first() - .find("span") - .last() - .text(), - ).toBe(`${firstName} ${lastName}`); - - // add admin permissions - const userGroupSelect = userRow.find(UserGroupSelect); - expect(userGroupSelect.text()).toBe("Default"); - click(userGroupSelect); - - click( - app - .find(".TestPopover") - .find(GroupOption) - .first(), - ); - await store.waitForActions([CREATE_MEMBERSHIP]); - - click(userRow.find(EntityMenuTrigger)); - click(app.find(EntityMenuItem).find('span[children="Edit user"]')); - - await store.waitForActions([BROWSER_HISTORY_PUSH]); - - const editDetailsModal = app.find(ModalContent); - - const saveButton = findButtonByText(app, "Update"); - - setInputValue( - editDetailsModal.find("input[name='first_name']"), - firstName + "x", - ); - setInputValue( - editDetailsModal.find("input[name='last_name']"), - lastName + "x", - ); - setInputValue(editDetailsModal.find("input[name='email']"), email + "x"); - expect(saveButton.props().disabled).toBe(false); - - clickButton(saveButton); - await store.waitForActions([ - "metabase/entities/users/UPDATE", - BROWSER_HISTORY_POP, - ]); - - const updatedUserRow = usersTable - .find(`td[children="${email}x"]`) - .closest("tr"); - expect( - updatedUserRow - .find("td") - .first() - .find("span") - .last() - .text(), - ).toBe(`${firstName}x ${lastName}x`); - - click(userRow.find(EntityMenuTrigger)); - click(app.find(EntityMenuItem).find('span[children="Reset password"]')); - - await store.waitForActions([BROWSER_HISTORY_PUSH]); - - const resetPasswordModal = app.find(ModalContent); - clickButton(resetPasswordModal.find(".Button.Button--danger")); - - await store.waitForActions([PASSWORD_RESET_MANUAL]); - - // this assumes no email configured - click(resetPasswordModal.find('a[children="Show"]')); - const newPassword = resetPasswordModal.find("input").prop("value"); - - expect(newPassword).not.toEqual(password); - }); - - afterAll(async () => { - // Test cleanup - if (createdUserId) { - await UserApi.delete({ userId: createdUserId }); - } - }); - }); -}); diff --git a/frontend/test/metabase/internal/__snapshots__/components.unit.spec.js.snap b/frontend/test/metabase/internal/__snapshots__/components.unit.spec.js.snap index 3a25ad2e62aa999007525d43a3d971a9580b8c4e..0fff2fd57b0c326f6650031040b3be6384fb8d6b 100644 --- a/frontend/test/metabase/internal/__snapshots__/components.unit.spec.js.snap +++ b/frontend/test/metabase/internal/__snapshots__/components.unit.spec.js.snap @@ -975,7 +975,7 @@ exports[`Select should render "default" correctly 1`] = ` style={undefined} > <div - className="full-width AdminSelect flex align-center" + className="flex-full AdminSelect flex align-center" style={undefined} > <span @@ -1012,7 +1012,7 @@ exports[`Select should render "kitchen_sink" correctly 1`] = ` style={undefined} > <div - className="full-width AdminSelect flex align-center" + className="flex-full AdminSelect flex align-center" style={undefined} > <span @@ -1053,7 +1053,7 @@ exports[`Select should render "multiple" correctly 1`] = ` style={undefined} > <div - className="full-width AdminSelect flex align-center" + className="flex-full AdminSelect flex align-center" style={undefined} > <span @@ -1094,7 +1094,7 @@ exports[`Select should render "search" correctly 1`] = ` style={undefined} > <div - className="full-width AdminSelect flex align-center" + className="flex-full AdminSelect flex align-center" style={undefined} > <span @@ -1131,7 +1131,7 @@ exports[`Select should render "sections" correctly 1`] = ` style={undefined} > <div - className="full-width AdminSelect flex align-center" + className="flex-full AdminSelect flex align-center" style={undefined} > <span diff --git a/frontend/test/metabase/lib/redux.unit.spec.js b/frontend/test/metabase/lib/redux.unit.spec.js index 048dd5162ea3a1ffac0cdff51b9ab6d0d9c99156..b2c4d7ff9d0660d5888a7a8b4d74b44c15e9504c 100644 --- a/frontend/test/metabase/lib/redux.unit.spec.js +++ b/frontend/test/metabase/lib/redux.unit.spec.js @@ -1,4 +1,4 @@ -import { fetchData, updateData } from "metabase/lib/redux"; +import { fetchData, updateData, mergeEntities } from "metabase/lib/redux"; import { delay } from "metabase/lib/promise"; @@ -141,4 +141,28 @@ describe("Metadata", () => { expect(data).toEqual(args.existingData); }); }); + + describe("mergeEntities", () => { + it("add an entity", () => { + expect( + mergeEntities( + { 1: { id: 1, name: "foo" } }, + { 2: { id: 2, name: "bar" } }, + ), + ).toEqual({ 1: { id: 1, name: "foo" }, 2: { id: 2, name: "bar" } }); + }); + it("merge entity keys", () => { + expect( + mergeEntities( + { 1: { id: 1, name: "foo", prop1: 123 } }, + { 1: { id: 1, name: "bar", prop2: 456 } }, + ), + ).toEqual({ 1: { id: 1, name: "bar", prop1: 123, prop2: 456 } }); + }); + it("delete an entity", () => { + expect( + mergeEntities({ 1: { id: 1 }, 2: { id: 2 } }, { 2: null }), + ).toEqual({ 1: { id: 1 } }); + }); + }); }); diff --git a/frontend/test/metabase/lib/utils.unit.spec.js b/frontend/test/metabase/lib/utils.unit.spec.js index 9f6ea368db66a787bdec0ff4302b971a9945ffe3..3fc3c928502a1d3a562e0b166449531c0a15519b 100644 --- a/frontend/test/metabase/lib/utils.unit.spec.js +++ b/frontend/test/metabase/lib/utils.unit.spec.js @@ -37,7 +37,7 @@ describe("utils", () => { it("can enforce uppercase requirements", () => { expect( - MetabaseUtils.generatePassword({ total: 14, uppercase: 2 }).match( + MetabaseUtils.generatePassword({ total: 14, upper: 2 }).match( /([A-Z])/g, ).length >= 2, ).toBe(true); diff --git a/frontend/test/metabase/parameters/components/Parameters.unit.spec.js b/frontend/test/metabase/parameters/components/Parameters.unit.spec.js index f3fd1f41ebbbcbd85a198f4a1fe3e95e0b756c6e..97ea01d93f8629c9f12a116503160969aabaed63 100644 --- a/frontend/test/metabase/parameters/components/Parameters.unit.spec.js +++ b/frontend/test/metabase/parameters/components/Parameters.unit.spec.js @@ -1,3 +1,5 @@ +import Field from "metabase-lib/lib/metadata/Field"; + import { ORDERS, PRODUCTS } from "__support__/sample_dataset_fixture"; import { parseQueryParam } from "metabase/parameters/components/Parameters"; @@ -24,5 +26,13 @@ describe("Parameters", () => { const result = parseQueryParam("123", []); expect(result).toBe("123"); }); + it("should not parse date/numeric fields", () => { + const dateField = new Field({ + ...ORDERS.QUANTITY, // some numeric field + special_type: "type/UNIXTimestampSeconds", // make it a date + }); + const result = parseQueryParam("past30days", [dateField]); + expect(result).toBe("past30days"); + }); }); }); diff --git a/frontend/test/metabase/redux/metadata.e2e.spec.js b/frontend/test/metabase/redux/metadata.e2e.spec.js deleted file mode 100644 index 789d5c9e7254fa018a64a34a3e6d785b18f16dfd..0000000000000000000000000000000000000000 --- a/frontend/test/metabase/redux/metadata.e2e.spec.js +++ /dev/null @@ -1,104 +0,0 @@ -/** - * Goals for this test suite: - * - serve as a documentation for how to use metadata loading actions - * - see if the metadata gets properly hydrated in `getMetadata` of selectors/metadata - * - see if metadata loading actions can be safely used in isolation from each others - */ -import { getMetadata } from "metabase/selectors/metadata"; -import { createTestStore, useSharedAdminLogin } from "__support__/e2e"; -import { fetchMetrics, fetchTables } from "metabase/redux/metadata"; - -const metadata = store => getMetadata(store.getState()); - -describe("metadata/redux", () => { - beforeAll(async () => { - useSharedAdminLogin(); - }); - - describe("METRIC ACTIONS", () => { - // TODO Atte Keinänen 6/23/17: Remove metrics after their creation in other tests - describe("fetchMetrics()", () => { - pending(); - it("fetches no metrics in empty db", async () => { - const store = createTestStore(); - await store.dispatch(fetchMetrics()); - expect(metadata(store).metricsList().length).toBe(0); - }); - }); - describe("updateMetric(metric)", () => { - // await store.dispatch(updateMetric(metric)); - }); - describe("updateMetricImportantFields(...)", () => { - // await store.dispatch(updateMetricImportantFields(metricId, importantFieldIds)); - }); - describe("fetchMetricTable(metricId)", () => { - // await store.dispatch(fetchMetricTable(metricId)); - }); - describe("fetchMetricRevisions(metricId)", () => { - // await store.dispatch(fetchMetricRevisions(metricId)); - }); - }); - - describe("SEGMENT ACTIONS", () => { - describe("fetchSegments()", () => { - // await store.dispatch(fetchSegments()); - }); - describe("updateSegment(segment)", () => { - // await store.dispatch(updateSegment(segment)); - }); - }); - - describe("DATABASE ACTIONS", () => { - describe("fetchDatabaseMetadata(dbId)", () => { - // await store.dispatch(fetchDatabaseMetadata(1)); - }); - describe("updateDatabase(database)", () => { - // await store.dispatch(updateDatabase(database)); - }); - }); - - describe("TABLE ACTIONS", () => { - describe("fetchTables()", () => { - // TODO Atte Keinänen 6/23/17: Figure out why on CI two databases show up but locally only one - pending(); - it("fetches the sample dataset tables", async () => { - // what is the difference between fetchDatabases and fetchTables? - const store = createTestStore(); - expect(metadata(store).tablesList().length).toBe(0); - expect(metadata(store).databasesList().length).toBe(0); - - await store.dispatch(fetchTables()); - expect(metadata(store).tablesList().length).toBe(4); - expect(metadata(store).databasesList().length).toBe(1); - }); - // await store.dispatch(fetchTables()); - }); - describe("updateTable(table)", () => { - // await store.dispatch(updateTable(table)); - }); - describe("fetchTableMetadata(tableId)", () => { - // await store.dispatch(fetchTableMetadata(tableId)); - }); - describe("fetchFieldValues(fieldId)", () => { - // await store.dispatch(fetchFieldValues()); - }); - }); - - describe("MISC ACTIONS", () => { - describe("addParamValues(paramValues)", () => { - // await store.dispatch(addParamValues(paramValues)); - }); - describe("updateField(field)", () => { - // await store.dispatch(updateField(field)); - }); - describe("fetchRevisions(type, id)", () => { - // await store.dispatch(fetchRevisions(type, id)); - }); - describe("fetchSegmentFields(segmentId)", () => { - // await store.dispatch(fetchSegmentFields(segmentId)); - }); - describe("fetchSegmentRevisions(segments)", () => { - // await store.dispatch(fetchSegmentRevisions(segmentId)); - }); - }); -}); diff --git a/frontend/test/metabase/reference/databases.e2e.spec.js b/frontend/test/metabase/reference/databases.e2e.spec.js deleted file mode 100644 index 4a232f9946643178bec6fef0c13985ea3c993f67..0000000000000000000000000000000000000000 --- a/frontend/test/metabase/reference/databases.e2e.spec.js +++ /dev/null @@ -1,249 +0,0 @@ -import { useSharedAdminLogin, createTestStore } from "__support__/e2e"; -import { click, clickButton, setInputValue } from "__support__/enzyme"; - -import React from "react"; -import { mount } from "enzyme"; - -import { CardApi, MetabaseApi } from "metabase/services"; - -import { - FETCH_DATABASE_METADATA, - FETCH_REAL_DATABASES, -} from "metabase/redux/metadata"; - -import { END_LOADING } from "metabase/reference/reference"; - -import DatabaseListContainer from "metabase/reference/databases/DatabaseListContainer"; -import DatabaseDetailContainer from "metabase/reference/databases/DatabaseDetailContainer"; -import TableListContainer from "metabase/reference/databases/TableListContainer"; -import TableDetailContainer from "metabase/reference/databases/TableDetailContainer"; -import TableQuestionsContainer from "metabase/reference/databases/TableQuestionsContainer"; -import FieldListContainer from "metabase/reference/databases/FieldListContainer"; -import FieldDetailContainer from "metabase/reference/databases/FieldDetailContainer"; - -import DatabaseList from "metabase/reference/databases/DatabaseList"; -import List from "metabase/components/List"; -import ListItem from "metabase/components/ListItem"; -import ReferenceHeader from "metabase/reference/components/ReferenceHeader"; -import AdminAwareEmptyState from "metabase/components/AdminAwareEmptyState"; -import UsefulQuestions from "metabase/reference/components/UsefulQuestions"; -import Detail from "metabase/reference/components/Detail"; -import QueryButton from "metabase/components/QueryButton"; -import { INITIALIZE_QB, QUERY_COMPLETED } from "metabase/query_builder/actions"; -import { getQuestion } from "metabase/query_builder/selectors"; -import { delay } from "metabase/lib/promise"; - -describe("The Reference Section", () => { - // Test data - const cardDef = { - name: "A card", - display: "scalar", - dataset_query: { - database: 1, - type: "query", - query: { "source-table": 1, aggregation: [["count"]] }, - }, - visualization_settings: {}, - }; - - // Scaffolding - beforeAll(async () => { - useSharedAdminLogin(); - }); - - describe("The Data Reference for the Sample Database", async () => { - // database list - it("should see databases", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/"); - const container = mount( - store.connectContainer(<DatabaseListContainer />), - ); - await store.waitForActions([FETCH_REAL_DATABASES, END_LOADING]); - - expect(container.find(ReferenceHeader).length).toBe(1); - expect(container.find(DatabaseList).length).toBe(1); - expect(container.find(AdminAwareEmptyState).length).toBe(0); - - expect(container.find(List).length).toBe(1); - expect(container.find(ListItem).length).toBeGreaterThanOrEqual(1); - }); - - // database list - it("should not see saved questions in the database list", async () => { - const card = await CardApi.create(cardDef); - const store = await createTestStore(); - store.pushPath("/reference/databases/"); - const container = mount( - store.connectContainer(<DatabaseListContainer />), - ); - await store.waitForActions([FETCH_REAL_DATABASES, END_LOADING]); - - expect(container.find(ReferenceHeader).length).toBe(1); - expect(container.find(DatabaseList).length).toBe(1); - expect(container.find(AdminAwareEmptyState).length).toBe(0); - - expect(container.find(List).length).toBe(1); - expect(container.find(ListItem).length).toBe(1); - - expect(card.name).toBe(cardDef.name); - - await CardApi.delete({ cardId: card.id }); - }); - - // database detail - it("should see a the detail view for the sample database", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1"); - mount(store.connectContainer(<DatabaseDetailContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - - // database update - it("should update the sample database", async () => { - // create a new db by cloning #1 - const d1 = await MetabaseApi.db_get({ dbId: 1 }); - const { id } = await MetabaseApi.db_create(d1); - - // go to that db's reference page - const store = await createTestStore(); - store.pushPath(`/reference/databases/${id}`); - const app = mount(store.connectContainer(<DatabaseDetailContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - - // switch to edit view - const editButton = app.find(".Button"); - - clickButton(editButton); - - // update "caveats" and save - const textarea = app - .find(Detail) - .at(2) - .find("textarea"); - setInputValue(textarea, "v important thing"); - - const doneButton = app.find(".Button--primary"); - - clickButton(doneButton); - await store.waitForActions(END_LOADING); - // unfortunately this is required? - await delay(200); - - // check that the field was updated - const savedText = app - .find(Detail) - .at(2) - .find("span") - .at(1) - .text(); - - expect(savedText).toBe("v important thing"); - - // clean up - await MetabaseApi.db_delete({ dbId: id }); - }); - - // table list - it("should see the 4 tables in the sample database", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables"); - mount(store.connectContainer(<TableListContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - // table detail - - it("should see the Orders table", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/1"); - mount(store.connectContainer(<TableDetailContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - - it("should see the Reviews table", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/2"); - mount(store.connectContainer(<TableDetailContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - it("should see the Products table", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/3"); - mount(store.connectContainer(<TableDetailContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - it("should see the People table", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/4"); - mount(store.connectContainer(<TableDetailContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - // field list - it("should see the fields for the orders table", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/1/fields"); - mount(store.connectContainer(<FieldListContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - it("should see the questions for the orders tables", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/1/questions"); - mount(store.connectContainer(<TableQuestionsContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - - const card = await CardApi.create(cardDef); - - expect(card.name).toBe(cardDef.name); - - await CardApi.delete({ cardId: card.id }); - }); - - // field detail - - it("should see the orders created_at timestamp field", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/1/fields/1"); - mount(store.connectContainer(<FieldDetailContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - - it("should let you open a potentially useful question for created_at field without errors", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/1/fields/1"); - - const app = mount(store.getAppContainer()); - - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - const fieldDetails = app.find(FieldDetailContainer); - expect(fieldDetails.length).toBe(1); - - const usefulQuestionLink = fieldDetails - .find(UsefulQuestions) - .find(QueryButton) - .first() - .find("a"); - expect(usefulQuestionLink.text()).toBe( - "Number of Orders grouped by Created At", - ); - click(usefulQuestionLink); - - await store.waitForActions([INITIALIZE_QB, QUERY_COMPLETED]); - - const qbQuery = getQuestion(store.getState()).query(); - - // the granularity/subdimension should be applied correctly to the breakout - expect(JSON.stringify(qbQuery.breakouts())).toEqual( - JSON.stringify([ - ["datetime-field", ["field-id", 1], "month"], // depends on the date range - ]), - ); - }); - - it("should see the orders id field", async () => { - const store = await createTestStore(); - store.pushPath("/reference/databases/1/tables/1/fields/25"); - mount(store.connectContainer(<FieldDetailContainer />)); - await store.waitForActions([FETCH_DATABASE_METADATA, END_LOADING]); - }); - }); -}); diff --git a/frontend/test/metabase/reference/metrics.e2e.spec.js b/frontend/test/metabase/reference/metrics.e2e.spec.js deleted file mode 100644 index a6b8a89865ab8b4be542ce3abd656eaeb6dbc5d1..0000000000000000000000000000000000000000 --- a/frontend/test/metabase/reference/metrics.e2e.spec.js +++ /dev/null @@ -1,143 +0,0 @@ -import { useSharedAdminLogin, createTestStore } from "__support__/e2e"; - -import React from "react"; -import { mount } from "enzyme"; -import { assocIn } from "icepick"; - -import { CardApi, MetricApi } from "metabase/services"; - -import { - FETCH_METRICS, - FETCH_METRIC_TABLE, - FETCH_METRIC_REVISIONS, -} from "metabase/redux/metadata"; - -import { FETCH_GUIDE } from "metabase/reference/reference"; - -import MetricListContainer from "metabase/reference/metrics/MetricListContainer"; -import MetricDetailContainer from "metabase/reference/metrics/MetricDetailContainer"; -import MetricQuestionsContainer from "metabase/reference/metrics/MetricQuestionsContainer"; -import MetricRevisionsContainer from "metabase/reference/metrics/MetricRevisionsContainer"; - -// NOTE: database/table_id/source-table are hard-coded, this might be a problem at some point - -describe("The Reference Section", () => { - // Test data - const metricDef = { - name: "A Metric", - description: "I did it!", - table_id: 1, - show_in_getting_started: true, - definition: { aggregation: [["count"]] }, - }; - - const anotherMetricDef = { - name: "Another Metric", - description: "I did it again!", - table_id: 1, - show_in_getting_started: true, - definition: { aggregation: [["count"]] }, - }; - - const metricCardDef = { - name: "A card", - display: "scalar", - dataset_query: { - database: 1, - type: "query", - query: { "source-table": 1, aggregation: [["metric", 1]] }, - }, - visualization_settings: {}, - }; - - // Scaffolding - beforeAll(async () => { - useSharedAdminLogin(); - }); - - describe("The Metrics section of the Data Reference", async () => { - describe("Empty State", async () => { - it("Should show no metrics in the list", async () => { - const store = await createTestStore(); - store.pushPath("/reference/metrics"); - mount(store.connectContainer(<MetricListContainer />)); - await store.waitForActions([FETCH_METRICS]); - }); - }); - - describe("With Metrics State", async () => { - const metricIds = []; - - beforeAll(async () => { - // Create some metrics to have something to look at - const metric = await MetricApi.create(metricDef); - const metric2 = await MetricApi.create(anotherMetricDef); - - metricIds.push(metric.id); - metricIds.push(metric2.id); - }); - - afterAll(async () => { - // Delete the guide we created - // remove the metrics we created - // This is a bit messy as technically these are just archived - for (const id of metricIds) { - await MetricApi.delete({ metricId: id, revision_message: "Please" }); - } - }); - // metrics list - it("Should show no metrics in the list", async () => { - const store = await createTestStore(); - store.pushPath("/reference/metrics"); - mount(store.connectContainer(<MetricListContainer />)); - await store.waitForActions([FETCH_METRICS]); - }); - // metric detail - it("Should show the metric detail view for a specific id", async () => { - const store = await createTestStore(); - store.pushPath("/reference/metrics/" + metricIds[0]); - mount(store.connectContainer(<MetricDetailContainer />)); - await store.waitForActions([FETCH_METRIC_TABLE, FETCH_GUIDE]); - }); - // metrics questions - it("Should show no questions based on a new metric", async () => { - const store = await createTestStore(); - store.pushPath("/reference/metrics/" + metricIds[0] + "/questions"); - mount(store.connectContainer(<MetricQuestionsContainer />)); - await store.waitForActions([FETCH_METRICS, FETCH_METRIC_TABLE]); - }); - // metrics revisions - it("Should show revisions", async () => { - const store = await createTestStore(); - store.pushPath("/reference/metrics/" + metricIds[0] + "/revisions"); - mount(store.connectContainer(<MetricRevisionsContainer />)); - await store.waitForActions([FETCH_METRICS, FETCH_METRIC_REVISIONS]); - }); - - it("Should see a newly asked question in its questions list", async () => { - let card; - try { - const cardDef = assocIn( - metricCardDef, - ["dataset_query", "query", "aggregation", 0, 1], - metricIds[0], - ); - card = await CardApi.create(cardDef); - expect(card.name).toBe(metricCardDef.name); - - // see that there is a new question on the metric's questions page - const store = await createTestStore(); - - store.pushPath("/reference/metrics/" + metricIds[0] + "/questions"); - mount(store.connectContainer(<MetricQuestionsContainer />)); - await store.waitForActions([FETCH_METRICS, FETCH_METRIC_TABLE]); - } finally { - if (card) { - // even if the code above results in an exception, try to delete the question - await CardApi.delete({ cardId: card.id }); - } - } - }); - }); - }); -}); diff --git a/frontend/test/metabase/scenarios/admin/datamodel/field.cy.spec.js b/frontend/test/metabase/scenarios/admin/datamodel/field.cy.spec.js index a51f4681d98c3fc7ac65a0c190048a2bcf75e84f..b798e8b0a1c98751f89dcad3dc3a53510e93511f 100644 --- a/frontend/test/metabase/scenarios/admin/datamodel/field.cy.spec.js +++ b/frontend/test/metabase/scenarios/admin/datamodel/field.cy.spec.js @@ -2,7 +2,9 @@ import { signInAsAdmin, restore, withSampleDataset, + withDatabase, visitAlias, + popover, } from "__support__/cypress"; describe("scenarios > admin > datamodel > field", () => { @@ -159,5 +161,38 @@ describe("scenarios > admin > datamodel > field", () => { cy.contains("Custom mapping"); cy.get('input[value="foo"]'); }); + + it("allows 'Custom mapping' null values", () => { + restore("withSqlite"); + signInAsAdmin(); + const dbId = 2; + withDatabase( + dbId, + ({ number_with_nulls: { num }, number_with_nulls_ID }) => + cy.visit( + `/admin/datamodel/database/${dbId}/table/${number_with_nulls_ID}/${num}/general`, + ), + ); + + // change to custom mapping + cy.findByText("Use original value").click(); + popover() + .findByText("Custom mapping") + .click(); + + // update text for nulls from "null" to "nothin" + cy.get("input[value=null]") + .clear() + .type("nothin"); + cy.findByText("Save").click(); + cy.findByText("Saved!"); + + // check that it appears in QB + cy.visit("/question/new"); + cy.findByText("Simple question").click(); + cy.findByText("sqlite").click(); + cy.findByText("Number With Nulls").click(); + cy.findByText("nothin"); + }); }); }); diff --git a/frontend/test/metabase/scenarios/admin/datamodel/metrics.cy.spec.js b/frontend/test/metabase/scenarios/admin/datamodel/metrics.cy.spec.js index 47fc9772a1836767e06f0abd6b3dc732703acf9c..69ef7020c0d3447d48b9a3b7c52177149048f89d 100644 --- a/frontend/test/metabase/scenarios/admin/datamodel/metrics.cy.spec.js +++ b/frontend/test/metabase/scenarios/admin/datamodel/metrics.cy.spec.js @@ -7,103 +7,165 @@ describe("scenarios > admin > datamodel > metrics", () => { cy.viewport(1400, 860); }); - it("should create a metric", () => { - cy.visit("/admin"); - cy.contains("Data Model").click(); - cy.contains("Metrics").click(); - cy.contains("New metric").click(); - cy.contains("Select a table").click(); - popover() - .contains("Orders") - .click({ force: true }); // this shouldn't be needed, but there were issues with reordering as loads happeend - - cy.url().should("match", /metric\/create$/); - cy.contains("Create Your Metric"); - - // filter to orders with total under 100 - cy.contains("Add filters").click(); - cy.contains("Total").click(); - cy.contains("Equal to").click(); - cy.contains("Less than").click(); - cy.get('[placeholder="Enter a number"]').type("100"); - popover() - .contains("Add filter") - .click(); - - // - cy.contains("Result: 12765"); - - // fill in name/description - cy.get('[name="name"]').type("orders <100"); - cy.get('[name="description"]').type( - "Count of orders with a total under $100.", - ); - - // saving bounces you back and you see new metric in the list - cy.contains("Save changes").click(); - cy.url().should("match", /datamodel\/metrics$/); - cy.contains("orders <100"); - cy.contains("Count, Filtered by Total"); + describe("with no metrics", () => { + it("should show no metrics in the list", () => { + cy.visit("/admin/datamodel/metrics"); + cy.findByText( + "Create metrics to add them to the Summarize dropdown in the query builder", + ); + }); + + it("should show how to create metrics", () => { + cy.visit("/reference/metrics"); + cy.findByText( + "Metrics are the official numbers that your team cares about", + ); + cy.findByText("Learn how to create metrics"); + }); }); - it("should update that metric", () => { - cy.visit("/admin"); - cy.contains("Data Model").click(); - cy.contains("Metrics").click(); - - cy.contains("orders <100") - .parent() - .parent() - .find(".Icon-ellipsis") - .click(); - cy.contains("Edit Metric").click(); - - // update the filter from "< 100" to "> 10" - cy.url().should("match", /metric\/1$/); - cy.contains("Edit Your Metric"); - cy.contains(/Total\s+is less than/).click(); - popover() - .contains("Less than") - .click(); - popover() - .contains("Greater than") - .click(); - popover() - .find("input") - .type("{SelectAll}10"); - popover() - .contains("Update filter") - .click(); - - // confirm that the preview updated - cy.contains("Result: 18703"); - - // update name and description, set a revision note, and save the update - cy.get('[name="name"]') - .clear() - .type("orders >10"); - cy.get('[name="description"]') - .clear() - .type("Count of orders with a total over $10."); - cy.get('[name="revision_message"]').type("time for a change"); - cy.contains("Save changes").click(); - - // get redirected to previous page and see the new metric name - cy.url().should("match", /datamodel\/metrics$/); - cy.contains("orders >10"); - - // clean up - cy.contains("orders >10") - .parent() - .parent() - .find(".Icon-ellipsis") - .click(); - cy.contains("Retire Metric").click(); - modal() - .find("textarea") - .type("delete it"); - modal() - .contains("button", "Retire") - .click(); + describe("with metrics", () => { + before(() => { + // CREATE METRIC + signInAsAdmin(); + cy.visit("/admin"); + cy.contains("Data Model").click(); + cy.contains("Metrics").click(); + cy.contains("New metric").click(); + cy.contains("Select a table").click(); + popover() + .contains("Orders") + .click({ force: true }); // this shouldn't be needed, but there were issues with reordering as loads happeend + + cy.url().should("match", /metric\/create$/); + cy.contains("Create Your Metric"); + + // filter to orders with total under 100 + cy.contains("Add filters").click(); + cy.contains("Total").click(); + cy.contains("Equal to").click(); + cy.contains("Less than").click(); + cy.get('[placeholder="Enter a number"]').type("100"); + popover() + .contains("Add filter") + .click(); + + // + cy.contains("Result: 12765"); + + // fill in name/description + cy.get('[name="name"]').type("orders <100"); + cy.get('[name="description"]').type( + "Count of orders with a total under $100.", + ); + + // saving bounces you back and you see new metric in the list + cy.contains("Save changes").click(); + cy.url().should("match", /datamodel\/metrics$/); + cy.contains("orders <100"); + cy.contains("Count, Filtered by Total"); + }); + + it("should show no questions based on a new metric", () => { + cy.visit("/reference/metrics/1/questions"); + cy.findAllByText("Questions about orders <100"); + cy.findByText("Loading..."); + cy.findByText("Loading...").should("not.exist"); + cy.findByText( + "Questions about this metric will appear here as they're added", + ); + }); + + it("should see a newly asked question in its questions list", () => { + // Ask a new qustion + cy.visit("/reference/metrics/1/questions"); + cy.get(".full") + .find(".Button") + .click(); + cy.findByText("Filter").click(); + cy.findByText("Total").click(); + cy.findByText("Equal to").click(); + cy.findByText("Greater than").click(); + cy.findByPlaceholderText("Enter a number").type("50"); + cy.findByText("Add filter").click(); + cy.findByText("Save").click(); + cy.findAllByText("Save") + .last() + .click(); + cy.findByText("Not now").click(); + + // Check the list + cy.visit("/reference/metrics/1/questions"); + cy.findByText("Our analysis").should("not.exist"); + cy.findAllByText("Questions about orders <100"); + cy.findByText("Orders, orders <100, Filtered by Total"); + }); + + it("should show the metric detail view for a specific id", () => { + cy.visit("/admin/datamodel/metric/1"); + cy.findByText("Edit Your Metric"); + cy.findByText("Preview"); + }); + + it("should update that metric", () => { + cy.visit("/admin"); + cy.contains("Data Model").click(); + cy.contains("Metrics").click(); + + cy.contains("orders <100") + .parent() + .parent() + .find(".Icon-ellipsis") + .click(); + cy.contains("Edit Metric").click(); + + // update the filter from "< 100" to "> 10" + cy.url().should("match", /metric\/1$/); + cy.contains("Edit Your Metric"); + cy.contains(/Total\s+is less than/).click(); + popover() + .contains("Less than") + .click(); + popover() + .contains("Greater than") + .click(); + popover() + .find("input") + .type("{SelectAll}10"); + popover() + .contains("Update filter") + .click(); + + // confirm that the preview updated + cy.contains("Result: 18703"); + + // update name and description, set a revision note, and save the update + cy.get('[name="name"]') + .clear() + .type("orders >10"); + cy.get('[name="description"]') + .clear() + .type("Count of orders with a total over $10."); + cy.get('[name="revision_message"]').type("time for a change"); + cy.contains("Save changes").click(); + + // get redirected to previous page and see the new metric name + cy.url().should("match", /datamodel\/metrics$/); + cy.contains("orders >10"); + + // clean up + cy.contains("orders >10") + .parent() + .parent() + .find(".Icon-ellipsis") + .click(); + cy.contains("Retire Metric").click(); + modal() + .find("textarea") + .type("delete it"); + modal() + .contains("button", "Retire") + .click(); + }); }); }); diff --git a/frontend/test/metabase/scenarios/dashboard/permissions.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/permissions.cy.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..c7319d10ad030f8c99a93f4aa4c2c0d7115b0193 --- /dev/null +++ b/frontend/test/metabase/scenarios/dashboard/permissions.cy.spec.js @@ -0,0 +1,117 @@ +import { + signInAsAdmin, + signIn, + withSampleDataset, + restore, +} from "__support__/cypress"; + +describe("scenarios > dashboard > permissions", () => { + before(restore); + let dashboardId; + + it("should let admins view all cards in a dashboard", () => { + // This first test creates a dashboard with two questions. + // One is in Our Analytics the other is in a more locked down collection. + signInAsAdmin(); + + // The setup is a bunch of nested API calls to create the questions, dashboard, dashcards, collections and link them all up together. + let firstQuestionId, secondQuestionId; + withSampleDataset(({ ORDERS_ID }) => { + cy.request("POST", "/api/collection", { + name: "locked down collection", + color: "#509EE3", + parent_id: null, + }).then(({ body: { id: collection_id } }) => { + // TODO - This will break if the default snapshot updates collections or groups. + // We should first request the current graph and then modify it. + cy.request("PUT", "/api/collection/graph", { + revision: 1, + groups: { + "1": { "6": "none", root: "none" }, + "2": { "6": "write", root: "write" }, + "3": { "6": "write", root: "write" }, + "4": { "6": "none", root: "write" }, + "5": { "6": "none", root: "none" }, + }, + }); + cy.request("POST", "/api/card", { + dataset_query: { + database: 1, + type: "native", + native: { query: "select 'foo'" }, + }, + display: "table", + visualization_settings: {}, + name: "First Question", + collection_id, + }).then(({ body: { id } }) => (firstQuestionId = id)); + }); + cy.request("POST", "/api/card", { + dataset_query: { + database: 1, + type: "native", + native: { query: "select 'bar'" }, + }, + display: "table", + visualization_settings: {}, + name: "Second Question", + collection_id: null, + }).then(({ body: { id } }) => (secondQuestionId = id)); + }); + + cy.request("POST", "/api/dashboard", { name: "dashboard" }).then( + ({ body: { id: dashId } }) => { + cy.request("POST", `/api/dashboard/${dashId}/cards`, { + cardId: firstQuestionId, + }).then(({ body: { id: dashCardIdA } }) => { + cy.request("POST", `/api/dashboard/${dashId}/cards`, { + cardId: secondQuestionId, + }).then(({ body: { id: dashCardIdB } }) => { + cy.request("PUT", `/api/dashboard/${dashId}/cards`, { + cards: [ + { + id: dashCardIdA, + card_id: firstQuestionId, + row: 0, + col: 0, + sizeX: 6, + sizeY: 6, + }, + { + id: dashCardIdB, + card_id: secondQuestionId, + row: 0, + col: 6, + sizeX: 6, + sizeY: 6, + }, + ], + }); + }); + }); + dashboardId = dashId; + cy.visit(`/dashboard/${dashId}`); + }, + ); + + // Admin can see both questions + cy.findByText("First Question"); + cy.findByText("foo"); + cy.findByText("Second Question"); + cy.findByText("bar"); + }); + + it("should display dashboards with some cards locked down", () => { + signIn("nodata"); + cy.visit(`/dashboard/${dashboardId}`); + cy.findByText("Sorry, you don't have permission to see this card."); + cy.findByText("Second Question"); + cy.findByText("bar"); + }); + + it("should display an error if they don't have perms for the dashboard", () => { + signIn("nocollection"); + cy.visit(`/dashboard/${dashboardId}`); + cy.findByText("Sorry, you don’t have permission to see that."); + }); +}); diff --git a/frontend/test/metabase/scenarios/question/filter.cy.spec.js b/frontend/test/metabase/scenarios/question/filter.cy.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..6ba5ff2f5028177127c8db484515288d0d2991c4 --- /dev/null +++ b/frontend/test/metabase/scenarios/question/filter.cy.spec.js @@ -0,0 +1,71 @@ +import { + signInAsAdmin, + restore, + openProductsTable, + popover, +} from "__support__/cypress"; + +describe("scenarios > question > filter", () => { + before(restore); + beforeEach(signInAsAdmin); + + it.skip("should load needed data (Issue #12985)", () => { + // Save a Question + openProductsTable(); + cy.findByText("Save").click(); + cy.findByPlaceholderText("What is the name of your card?") + .clear() + .type("Q1"); + cy.findAllByText("Save") + .last() + .click(); + cy.findByText("Not now").click(); + + // From Q1, save Q2 + cy.visit("/question/new"); + cy.findByText("Simple question").click(); + cy.findByText("Saved Questions").click(); + cy.findByText("Q1").click(); + cy.findByText("Save").click(); + cy.findByPlaceholderText("What is the name of your card?") + .clear() + .type("Q2"); + cy.findAllByText("Save") + .last() + .click(); + + // Add Q2 to a dashboard + cy.findByText("Yes please!").click(); + cy.get(".Icon-dashboard").click(); + + // Add two dashboard filters + cy.get(".Icon-funnel_add").click(); + cy.findByText("Time").click(); + cy.findByText("All Options").click(); + cy.findAllByText("Select…") + .last() + .click(); + cy.findByText("Created At").click(); + + cy.get(".Icon-funnel_add").click(); + cy.findByText("Other Categories").click(); + cy.findAllByText("Select…") + .last() + .click(); + popover().within(() => { + cy.findByText("Category").click(); + }); + + // Save dashboard and refresh page + cy.findByText("Done").click(); + cy.findByText("You are editing a dashboard"); + cy.findByText("Save").click(); + cy.findByText("Save").should("not.exist"); + + // Check category search + cy.get(".Icon-empty").should("not.exist"); + cy.findByText("Category").click(); + cy.findByText("Gadget").click(); + cy.findByText("Add filter").click(); + }); +}); diff --git a/frontend/test/metabase/scenarios/question/native.cy.spec.js b/frontend/test/metabase/scenarios/question/native.cy.spec.js index cca6f1b4cf20d88f1b63ca2c44eb8752b2ecb46d..9f23e2d704d124982b882c265e177e441ab0c6d5 100644 --- a/frontend/test/metabase/scenarios/question/native.cy.spec.js +++ b/frontend/test/metabase/scenarios/question/native.cy.spec.js @@ -1,6 +1,5 @@ import { signInAsNormalUser, - signInAsAdmin, restore, popover, modal, @@ -158,35 +157,6 @@ describe("scenarios > question > native", () => { cy.contains("18,760"); }); - it("should let you create and use a snippet", () => { - signInAsAdmin(); - cy.visit("/question/new"); - cy.contains("Native query").click(); - - // type a query and highlight some of the text - cy.get(".ace_content").as("ace"); - cy.get("@ace").type( - "select 'stuff'" + "{shift}{leftarrow}".repeat("'stuff'".length), - ); - - // add a snippet of that text - cy.get(".Icon-snippet").click(); - cy.contains("Create a snippet").click(); - modal() - .find("input[name=name]") - .type("stuff-snippet"); - modal() - .contains("Save") - .click(); - - // SQL editor should get updated automatically - cy.get("@ace").contains("select {{snippet: stuff-snippet}}"); - - // run the query and check the displayed scalar - cy.get(".NativeQueryEditor .Icon-play").click(); - cy.get(".ScalarValue").contains("stuff"); - }); - it("can load a question with a date filter (from issue metabase#12228)", () => { withSampleDataset(({ ORDERS }) => { cy.request("POST", "/api/card", { diff --git a/frontend/test/metabase/scenarios/question/notebook.cy.spec.js b/frontend/test/metabase/scenarios/question/notebook.cy.spec.js index a7c934920af4e1117cb2bdf6a2e3485eefeb36ef..308903879b3465770d5f901f8c094025c2f359b0 100644 --- a/frontend/test/metabase/scenarios/question/notebook.cy.spec.js +++ b/frontend/test/metabase/scenarios/question/notebook.cy.spec.js @@ -1,4 +1,10 @@ -import { restore, signInAsAdmin, popover, modal } from "__support__/cypress"; +import { + createNativeQuestion, + restore, + signInAsAdmin, + popover, + modal, +} from "__support__/cypress"; describe("scenarios > question > notebook", () => { before(restore); @@ -31,63 +37,98 @@ describe("scenarios > question > notebook", () => { cy.contains("Showing 1 row"); // ensure only one user was returned }); - it("should allow joins", () => { - // start a custom question with orders - cy.visit("/question/new"); - cy.contains("Custom question").click(); - cy.contains("Sample Dataset").click(); - cy.contains("Orders").click(); + describe("joins", () => { + it("should allow joins", () => { + // start a custom question with orders + cy.visit("/question/new"); + cy.contains("Custom question").click(); + cy.contains("Sample Dataset").click(); + cy.contains("Orders").click(); + + // join to Reviews on orders.product_id = reviews.product_id + cy.get(".Icon-join_left_outer").click(); + popover() + .contains("Reviews") + .click(); + popover() + .contains("Product ID") + .click(); + popover() + .contains("Product ID") + .click(); + + // get the average rating across all rows (not a useful metric) + cy.contains("Pick the metric you want to see").click(); + popover() + .contains("Average of") + .click(); + popover() + .find(".Icon-join_left_outer") + .click(); + popover() + .contains("Rating") + .click(); + cy.contains("Visualize").click(); + cy.contains("Orders + Reviews"); + cy.contains("3"); + }); - // join to Reviews on orders.product_id = reviews.product_id - cy.get(".Icon-join_left_outer").click(); - popover() - .contains("Reviews") - .click(); - popover() - .contains("Product ID") - .click(); - popover() - .contains("Product ID") - .click(); - - // get the average rating across all rows (not a useful metric) - cy.contains("Pick the metric you want to see").click(); - popover() - .contains("Average of") - .click(); - popover() - .find(".Icon-join_left_outer") - .click(); - popover() - .contains("Rating") - .click(); - cy.contains("Visualize").click(); - cy.contains("Orders + Reviews"); - cy.contains("3"); - }); + it("should allow post-join filters (metabase#12221)", () => { + cy.log("start a custom question with Orders"); + cy.visit("/question/new"); + cy.contains("Custom question").click(); + cy.contains("Sample Dataset").click(); + cy.contains("Orders").click(); + + cy.log("join to People table using default settings"); + cy.get(".Icon-join_left_outer ").click(); + cy.contains("People").click(); + cy.contains("Orders + People"); + cy.contains("Visualize").click(); + cy.contains("Showing first 2,000"); + + cy.log("attempt to filter on the joined table"); + cy.contains("Filter").click(); + cy.contains("Email").click(); + cy.contains("People – Email"); + cy.get('[placeholder="Search by Email"]').type("wolf."); + cy.contains("wolf.dina@yahoo.com").click(); + cy.contains("Add filter").click(); + cy.contains("Showing 1 row"); + }); - it("should allow post-join filters (metabase#12221)", () => { - cy.log("start a custom question with Orders"); - cy.visit("/question/new"); - cy.contains("Custom question").click(); - cy.contains("Sample Dataset").click(); - cy.contains("Orders").click(); + it("should join on field literals", () => { + // create two native questions + createNativeQuestion("question a", "select 'foo' as a_column"); + createNativeQuestion("question b", "select 'foo' as b_column"); - cy.log("join to People table using default settings"); - cy.get(".Icon-join_left_outer ").click(); - cy.contains("People").click(); - cy.contains("Orders + People"); - cy.contains("Visualize").click(); - cy.contains("Showing first 2,000"); - - cy.log("attempt to filter on the joined table"); - cy.contains("Filter").click(); - cy.contains("Email").click(); - cy.contains("People – Email"); - cy.get('[placeholder="Search by Email"]').type("wolf."); - cy.contains("wolf.dina@yahoo.com").click(); - cy.contains("Add filter").click(); - cy.contains("Showing 1 row"); + // start a custom question with question a + cy.visit("/question/new"); + cy.findByText("Custom question").click(); + cy.findByText("Saved Questions").click(); + cy.findByText("question a").click(); + + // join to question b + cy.get(".Icon-join_left_outer").click(); + popover().within(() => { + cy.findByText("Sample Dataset").click(); + cy.findByText("Saved Questions").click(); + cy.findByText("question b").click(); + }); + + // select the join columns + popover().within(() => cy.findByText("A_COLUMN").click()); + popover().within(() => cy.findByText("B_COLUMN").click()); + + cy.findByText("Visualize").click(); + cy.queryByText("Visualize").then($el => cy.wrap($el).should("not.exist")); // wait for that screen to disappear to avoid "multiple elements" errors + + // check that query worked + cy.findByText("question a + question b"); + cy.findByText("A_COLUMN"); + cy.findByText("Question 5 → B Column"); + cy.findByText("Showing 1 row"); + }); }); describe("nested", () => { diff --git a/frontend/test/metabase/scenarios/question/saved.cy.spec.js b/frontend/test/metabase/scenarios/question/saved.cy.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..4e9bdccc8a21024f03c6b6dbfdc5d5f92451bc04 --- /dev/null +++ b/frontend/test/metabase/scenarios/question/saved.cy.spec.js @@ -0,0 +1,40 @@ +import { + restore, + signInAsNormalUser, + popover, + modal, +} from "__support__/cypress"; + +describe("scenarios > question > saved", () => { + before(restore); + beforeEach(signInAsNormalUser); + + it("view and filter saved question", () => { + cy.visit("/question/1"); + cy.findAllByText("Orders"); // question and table name appears + + // filter to only orders with quantity=100 + cy.findByText("Quantity").click(); + popover().within(() => cy.findByText("Filter").click()); + popover().within(() => { + cy.findByPlaceholderText("Search the list").type("100"); + cy.findByText("Update filter").click(); + }); + cy.findByText("Quantity is equal to 100"); + cy.findByText("Showing 2 rows"); // query updated + + // check that save will give option to replace + cy.findByText("Save").click(); + modal().within(() => { + cy.findByText('Replace original question, "Orders"'); + cy.findByText("Save as new question"); + cy.findByText("Cancel").click(); + }); + + // click "Started from Orders" and check that the original question is restored + cy.findByText("Started from").within(() => cy.findByText("Orders").click()); + cy.findByText("Showing first 2,000 rows"); // query updated + cy.findByText("Started from").should("not.exist"); + cy.findByText("Quantity is equal to 100").should("not.exist"); + }); +}); diff --git a/frontend/test/metabase/scenarios/question/snippets.cy.spec.js b/frontend/test/metabase/scenarios/question/snippets.cy.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..1c233a83de3f212e0cf7aa79ca73b7ca1fbeb775 --- /dev/null +++ b/frontend/test/metabase/scenarios/question/snippets.cy.spec.js @@ -0,0 +1,59 @@ +import { signInAsNormalUser, restore, modal } from "__support__/cypress"; + +describe("scenarios > question > snippets", () => { + before(restore); + beforeEach(signInAsNormalUser); + + it("should let you create and use a snippet", () => { + cy.visit("/question/new"); + cy.contains("Native query").click(); + + // type a query and highlight some of the text + cy.get(".ace_content").as("ace"); + cy.get("@ace").type( + "select 'stuff'" + "{shift}{leftarrow}".repeat("'stuff'".length), + ); + + // add a snippet of that text + cy.get(".Icon-snippet").click(); + cy.contains("Create a snippet").click(); + modal() + .find("input[name=name]") + .type("stuff-snippet"); + modal() + .contains("Save") + .click(); + + // SQL editor should get updated automatically + cy.get("@ace").contains("select {{snippet: stuff-snippet}}"); + + // run the query and check the displayed scalar + cy.get(".NativeQueryEditor .Icon-play").click(); + cy.get(".ScalarValue").contains("stuff"); + }); + + it("should let you edit snippet", () => { + // open the snippet edit modal + cy.get(".Icon-chevrondown").click({ force: true }); + cy.findByText("Edit").click(); + + // update the name and content + modal().within(() => { + cy.findByText("Editing stuff-snippet"); + cy.findByLabelText("Enter some SQL here so you can reuse it later").type( + "{selectall}{del}'foo'", + ); + cy.findByLabelText("Give your snippet a name").type( + "{selectall}{del}foo", + ); + cy.findByText("Save").click(); + }); + + // SQL editor should get updated automatically + cy.get(".ace_content").contains("select {{snippet: foo}}"); + + // run the query and check the displayed scalar + cy.get(".NativeQueryEditor .Icon-play").click(); + cy.get(".ScalarValue").contains("foo"); + }); +}); diff --git a/frontend/test/snapshot-creators/default.cy.snap.js b/frontend/test/snapshot-creators/default.cy.snap.js index 9af881e279422f80f2642ad5429afb0bcf2c3e8e..61b643b4c781e1c3168e8942c22c81614638bb00 100644 --- a/frontend/test/snapshot-creators/default.cy.snap.js +++ b/frontend/test/snapshot-creators/default.cy.snap.js @@ -3,156 +3,198 @@ import { restore, USERS, withSampleDataset, + signInAsAdmin, } from "__support__/cypress"; -describe("default", () => { - it("default", () => { - snapshot("blank"); - setup(); - updateSettings(); - addUsersAndGroups(); - withSampleDataset(SAMPLE_DATASET => { - createQuestionAndDashboard(SAMPLE_DATASET); +describe("snapshots", () => { + describe("default", () => { + it("default", () => { + snapshot("blank"); + setup(); + updateSettings(); + addUsersAndGroups(); + withSampleDataset(SAMPLE_DATASET => { + createQuestionAndDashboard(SAMPLE_DATASET); + }); + snapshot("default"); + restore("blank"); }); - snapshot("default"); - restore("blank"); }); -}); -function makeUserObject(name, groupIds) { - return { - first_name: USERS[name].first_name, - last_name: USERS[name].last_name, - email: USERS[name].username, - password: USERS[name].password, - group_ids: groupIds, - }; -} - -function setup() { - cy.request("GET", "/api/session/properties").then(({ body: properties }) => { - cy.request("POST", "/api/setup", { - token: properties["setup-token"], - user: makeUserObject("admin"), - prefs: { - site_name: "Epic Team", - allow_tracking: false, + function makeUserObject(name, groupIds) { + return { + first_name: USERS[name].first_name, + last_name: USERS[name].last_name, + email: USERS[name].username, + password: USERS[name].password, + group_ids: groupIds, + }; + } + + function setup() { + cy.request("GET", "/api/session/properties").then( + ({ body: properties }) => { + cy.request("POST", "/api/setup", { + token: properties["setup-token"], + user: makeUserObject("admin"), + prefs: { + site_name: "Epic Team", + allow_tracking: false, + }, + database: null, + }); }, - database: null, + ); + } + + function updateSettings() { + cy.request("PUT", "/api/setting/enable-public-sharing", { value: true }); + cy.request("PUT", "/api/setting/enable-embedding", { value: true }); + cy.request("PUT", "/api/setting/embedding-secret-key", { + value: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", }); - }); -} -function updateSettings() { - cy.request("PUT", "/api/setting/enable-public-sharing", { value: true }); - cy.request("PUT", "/api/setting/enable-embedding", { value: true }); - cy.request("PUT", "/api/setting/embedding-secret-key", { - value: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", - }); + // update the Sample db connection string so it is valid in both CI and locally + cy.request("GET", "/api/database/1").then(response => { + response.body.details.db = + "./resources/sample-dataset.db;USER=GUEST;PASSWORD=guest"; + cy.request("PUT", "/api/database/1", response.body); + }); + } - // update the Sample db connection string so it is valid in both CI and locally - cy.request("GET", "/api/database/1").then(response => { - response.body.details.db = - "./resources/sample-dataset.db;USER=GUEST;PASSWORD=guest"; - cy.request("PUT", "/api/database/1", response.body); - }); -} - -const ALL_USERS_GROUP = 1; -const COLLECTION_GROUP = 4; -const DATA_GROUP = 5; - -function addUsersAndGroups() { - // groups - cy.request("POST", "/api/permissions/group", { name: "collection" }); // 4 - cy.request("POST", "/api/permissions/group", { name: "data" }); // 5 - - // additional users - cy.request( - "POST", - "/api/user", - makeUserObject("normal", [ALL_USERS_GROUP, COLLECTION_GROUP, DATA_GROUP]), - ); - cy.request( - "POST", - "/api/user", - makeUserObject("nodata", [ALL_USERS_GROUP, COLLECTION_GROUP]), - ); - cy.request( - "POST", - "/api/user", - makeUserObject("nocollection", [ALL_USERS_GROUP, DATA_GROUP]), - ); - cy.request("POST", "/api/user", makeUserObject("none", [ALL_USERS_GROUP])); - - // Make a call to `/api/user` because some things (personal collections) get created there - cy.request("GET", "/api/user"); - - // permissions - cy.request("PUT", "/api/permissions/graph", { - revision: 0, - groups: { - [ALL_USERS_GROUP]: { "1": { schemas: "none", native: "none" } }, - [DATA_GROUP]: { "1": { schemas: "all", native: "write" } }, - [COLLECTION_GROUP]: { "1": { schemas: "none", native: "none" } }, - }, - }); - cy.request("PUT", "/api/collection/graph", { - revision: 0, - groups: { - [ALL_USERS_GROUP]: { root: "none" }, - [DATA_GROUP]: { root: "none" }, - [COLLECTION_GROUP]: { root: "write" }, - }, - }); -} - -function createQuestionAndDashboard({ ORDERS, ORDERS_ID }) { - // question 1: Orders - cy.request("POST", "/api/card", { - name: "Orders", - display: "table", - visualization_settings: {}, - dataset_query: { - database: 1, - query: { "source-table": ORDERS_ID }, - type: "query", - }, - }); + const ALL_USERS_GROUP = 1; + const COLLECTION_GROUP = 4; + const DATA_GROUP = 5; - // question 2: Orders, Count - cy.request("POST", "/api/card", { - name: "Orders, Count", - display: "table", - visualization_settings: {}, - dataset_query: { - database: 1, - query: { "source-table": ORDERS_ID, aggregation: [["count"]] }, - type: "query", - }, - }); + function addUsersAndGroups() { + // groups + cy.request("POST", "/api/permissions/group", { name: "collection" }); // 4 + cy.request("POST", "/api/permissions/group", { name: "data" }); // 5 + + // additional users + cy.request( + "POST", + "/api/user", + makeUserObject("normal", [ALL_USERS_GROUP, COLLECTION_GROUP, DATA_GROUP]), + ); + cy.request( + "POST", + "/api/user", + makeUserObject("nodata", [ALL_USERS_GROUP, COLLECTION_GROUP]), + ); + cy.request( + "POST", + "/api/user", + makeUserObject("nocollection", [ALL_USERS_GROUP, DATA_GROUP]), + ); + cy.request("POST", "/api/user", makeUserObject("none", [ALL_USERS_GROUP])); - cy.request("POST", "/api/card", { - name: "Orders, Count, Grouped by Created At (year)", - dataset_query: { - type: "query", - query: { - "source-table": ORDERS_ID, - aggregation: [["count"]], - breakout: [["datetime-field", ["field-id", ORDERS.CREATED_AT], "year"]], + // Make a call to `/api/user` because some things (personal collections) get created there + cy.request("GET", "/api/user"); + + // permissions + cy.request("PUT", "/api/permissions/graph", { + revision: 0, + groups: { + [ALL_USERS_GROUP]: { "1": { schemas: "none", native: "none" } }, + [DATA_GROUP]: { "1": { schemas: "all", native: "write" } }, + [COLLECTION_GROUP]: { "1": { schemas: "none", native: "none" } }, }, - database: 1, - }, - display: "line", - visualization_settings: {}, - }); + }); + cy.request("PUT", "/api/collection/graph", { + revision: 0, + groups: { + [ALL_USERS_GROUP]: { root: "none" }, + [DATA_GROUP]: { root: "none" }, + [COLLECTION_GROUP]: { root: "write" }, + }, + }); + } + + function createQuestionAndDashboard({ ORDERS, ORDERS_ID }) { + // question 1: Orders + cy.request("POST", "/api/card", { + name: "Orders", + display: "table", + visualization_settings: {}, + dataset_query: { + database: 1, + query: { "source-table": ORDERS_ID }, + type: "query", + }, + }); - // dashboard 1: Orders in a dashboard - cy.request("POST", "/api/dashboard", { name: "Orders in a dashboard" }); - cy.request("POST", `/api/dashboard/1/cards`, { cardId: 1 }); + // question 2: Orders, Count + cy.request("POST", "/api/card", { + name: "Orders, Count", + display: "table", + visualization_settings: {}, + dataset_query: { + database: 1, + query: { "source-table": ORDERS_ID, aggregation: [["count"]] }, + type: "query", + }, + }); - // dismiss the "it's ok to play around" modal - Object.values(USERS).map((_, index) => - cy.request("PUT", `/api/user/${index + 1}/qbnewb`, {}), - ); -} + cy.request("POST", "/api/card", { + name: "Orders, Count, Grouped by Created At (year)", + dataset_query: { + type: "query", + query: { + "source-table": ORDERS_ID, + aggregation: [["count"]], + breakout: [ + ["datetime-field", ["field-id", ORDERS.CREATED_AT], "year"], + ], + }, + database: 1, + }, + display: "line", + visualization_settings: {}, + }); + + // dashboard 1: Orders in a dashboard + cy.request("POST", "/api/dashboard", { name: "Orders in a dashboard" }); + cy.request("POST", `/api/dashboard/1/cards`, { cardId: 1 }); + + // dismiss the "it's ok to play around" modal + Object.values(USERS).map((_, index) => + cy.request("PUT", `/api/user/${index + 1}/qbnewb`, {}), + ); + } + + // TODO: It'd be nice to have one file per snapshot. + // To do that we need to enforce execution order among them. + describe("withSqlite", () => { + it("withSqlite", () => { + restore("default"); + signInAsAdmin(); + cy.request("POST", "/api/database", { + engine: "sqlite", + name: "sqlite", + details: { db: "./resources/sqlite-fixture.db" }, + auto_run_queries: true, + is_full_sync: true, + schedules: { + cache_field_values: { + schedule_day: null, + schedule_frame: null, + schedule_hour: 0, + schedule_type: "daily", + }, + metadata_sync: { + schedule_day: null, + schedule_frame: null, + schedule_hour: null, + schedule_type: "hourly", + }, + }, + }); + cy.request("POST", "/api/database/2/sync_schema"); + cy.request("POST", "/api/database/2/rescan_values"); + cy.wait(1000); // wait for sync + snapshot("withSqlite"); + restore("blank"); + }); + }); +}); diff --git a/modules/drivers/redshift/src/metabase/driver/redshift.clj b/modules/drivers/redshift/src/metabase/driver/redshift.clj index 08cb2177856437737cb2e5d689b564efcf9f6aed..d7c38e04cda03ae4b848dac75bfb7ecd676f6d0a 100644 --- a/modules/drivers/redshift/src/metabase/driver/redshift.clj +++ b/modules/drivers/redshift/src/metabase/driver/redshift.clj @@ -175,32 +175,27 @@ [::legacy/use-legacy-classes-for-read-and-set OffsetTime] [:postgres OffsetTime]) -(defn query->field-values - "Convert a MBQL query to a map of field->value" - [query] +(defn- field->parameter-value + "Map fields used in parameters to parameter `:value`s." + [{:keys [user-parameters]}] (into {} - (filter identity - (map - (fn [param] - (if (contains? param :name) - [(:name param) (:value param)] - - (when-let [field-id (mbql.u/match-one - param - [:dimension field] - (let [field-id-or-name (mbql.u/field-clause->id-or-literal field)] - (when (integer? field-id-or-name) - field-id-or-name)))] - [(:name (qp.store/field field-id)) (:value param)]))) - (:user-parameters query))))) + (keep (fn [param] + (if (contains? param :name) + [(:name param) (:value param)] + + (when-let [field-id (mbql.u/match-one param + [:field-id field-id] (when (contains? (set &parents) :dimension) + field-id))] + [(:name (qp.store/field field-id)) (:value param)])))) + user-parameters)) (defmethod qputil/query->remark :redshift - [_ {{:keys [executed-by query-hash card-id], :as info} :info, query-type :type :as query}] + [_ {{:keys [executed-by query-hash card-id]} :info, :as query}] (str "/* partner: \"metabase\", " - (json/generate-string {:dashboard_id nil ;; requires metabase/metabase#11909 - :chart_id card-id - :optional_user_id executed-by + (json/generate-string {:dashboard_id nil ;; requires metabase/metabase#11909 + :chart_id card-id + :optional_user_id executed-by :optional_account_id (pubset/site-uuid) - :filter_values (query->field-values query)}) + :filter_values (field->parameter-value query)}) " */ " (qputil/default-query->remark query))) diff --git a/modules/drivers/redshift/test/metabase/driver/redshift_test.clj b/modules/drivers/redshift/test/metabase/driver/redshift_test.clj index 3494eba11dfe5fb19857fc5ac47687923db109aa..3ae404cf51718e9a8c3ea8ab2395d4c25589b7e5 100644 --- a/modules/drivers/redshift/test/metabase/driver/redshift_test.clj +++ b/modules/drivers/redshift/test/metabase/driver/redshift_test.clj @@ -131,3 +131,19 @@ [:asc $salesid]] :filter [:= [:field-id (mt/id :extsales :buyerid)] 11498]})) [:data :cols]))))))) + +(deftest parameters-test + (testing "Native query parameters should work with filters." + (is (= [[693 "2015-12-29T00:00:00Z" 10 90]] + (mt/rows + (qp/process-query + {:database (mt/id) + :type :native + :native {:query "select * from checkins where {{date}} order by date desc limit 1;" + :template-tags {"date" {:name "date" + :display-name "date" + :type :dimension + :dimension [:field-id (mt/id :checkins :date)]}}} + :parameters [{:type :date/all-options + :target [:dimension [:template-tag "date"]] + :value "past30years"}]})))))) diff --git a/modules/drivers/snowflake/resources/metabase-plugin.yaml b/modules/drivers/snowflake/resources/metabase-plugin.yaml index 00639566aa8f6c97b19c76b41e055df344a19ba7..39f3e80f047e5a3246cf957a1f5beec5b0128d58 100644 --- a/modules/drivers/snowflake/resources/metabase-plugin.yaml +++ b/modules/drivers/snowflake/resources/metabase-plugin.yaml @@ -24,6 +24,7 @@ driver: - dbname - name: db required: true + display-name: Database name (case sensitive) - name: regionid display-name: Region ID placeholder: my_region diff --git a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj index 1b75ba03d621a848f6b1da8541d3159fddc8743d..66b2509c94b8677eaa1ff6548b6c50543dc02860 100644 --- a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj +++ b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj @@ -28,16 +28,26 @@ [i18n :refer [tru]]]) (:import [java.sql ResultSet Types] [java.time OffsetDateTime ZonedDateTime] - metabase.util.honeysql_extensions.Identifier - net.snowflake.client.jdbc.SnowflakeSQLException)) + metabase.util.honeysql_extensions.Identifier)) (driver/register! :snowflake, :parent #{:sql-jdbc ::legacy/use-legacy-classes-for-read-and-set}) +(defmethod driver/humanize-connection-error-message :snowflake + [_ message] + (log/spy :error (type message)) + (condp re-matches message + #"(?s).*Object does not exist.*$" + (driver.common/connection-error-messages :database-name-incorrect) + + #"(?s).*" ; default - the Snowflake errors have a \n in them + message)) + (defmethod sql-jdbc.conn/connection-details->spec :snowflake [_ {:keys [account regionid], :as opts}] (let [host (if regionid (str account "." regionid) - account)] + account) + upcase-not-nil (fn [s] (when s (u/upper-case-en s)))] ;; it appears to be the case that their JDBC driver ignores `db` -- see my bug report at ;; https://support.snowflake.net/s/question/0D50Z00008WTOMCSA5/ (-> (merge {:classname "net.snowflake.client.jdbc.SnowflakeDriver" @@ -58,6 +68,9 @@ ;; original version of the Snowflake driver incorrectly used `dbname` in the details fields instead of ;; `db`. If we run across `dbname`, correct our behavior (set/rename-keys {:dbname :db}) + ;; see https://github.com/metabase/metabase/issues/9511 + (update :warehouse upcase-not-nil) + (update :schema upcase-not-nil) (dissoc :host :port :timezone))) (sql-jdbc.common/handle-additional-options opts)))) @@ -273,12 +286,8 @@ (and ((get-method driver/can-connect? :sql-jdbc) driver details) (let [spec (sql-jdbc.conn/details->connection-spec-for-testing-connection driver details) sql (format "SHOW OBJECTS IN DATABASE \"%s\";" db)] - (try - (jdbc/query spec sql) - true - (catch SnowflakeSQLException e - (log/error e (tru "Snowflake Database does not exist.")) - false))))) + (jdbc/query spec sql) + true))) (defmethod unprepare/unprepare-value [:snowflake OffsetDateTime] [_ t] diff --git a/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj b/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj index 2dcdf1b9c707cfef3edee984a8d6521bbcc1cb5a..35ac91e71ba3ce2a35f223ed7233216dbf0cc632 100644 --- a/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj +++ b/modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj @@ -129,10 +129,10 @@ (is (= true (can-connect? (:details (mt/db)))) "can-connect? should return true for normal Snowflake DB details") - (is (= false - (mt/suppress-output - (can-connect? (assoc (:details (mt/db)) :db (mt/random-name))))) - "can-connect? should return false for Snowflake databases that don't exist (#9041)")))) + (is (thrown? net.snowflake.client.jdbc.SnowflakeSQLException + (mt/suppress-output + (can-connect? (assoc (:details (mt/db)) :db (mt/random-name))))) + "can-connect? should throw for Snowflake databases that don't exist (#9511)")))) (deftest report-timezone-test (mt/test-driver :snowflake diff --git a/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj b/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj index edb20884a46a7b27beed37039c3327a4dbd48f24..8b621ee740a70cbb8b881bf001f4e142deb616bb 100644 --- a/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj +++ b/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj @@ -45,13 +45,15 @@ {:account (tx/db-test-env-var-or-throw :snowflake :account) :user (tx/db-test-env-var-or-throw :snowflake :user) :password (tx/db-test-env-var-or-throw :snowflake :password) - :warehouse (tx/db-test-env-var-or-throw :snowflake :warehouse) + ;; this lowercasing this value is part of testing the fix for + ;; https://github.com/metabase/metabase/issues/9511 + :warehouse (u/lower-case-en (tx/db-test-env-var-or-throw :snowflake :warehouse)) ;; SESSION parameters :timezone "UTC"} ;; Snowflake JDBC driver ignores this, but we do use it in the `query-db-name` function in ;; `metabase.driver.snowflake` (when (= context :db) - {:db (qualified-db-name database-name)}))) + {:db (qualified-db-name (u/lower-case-en database-name))}))) ;; Snowflake requires you identify an object with db-name.schema-name.table-name (defmethod sql.tx/qualified-name-components :snowflake @@ -74,7 +76,7 @@ (jdbc/with-db-metadata [metadata db-spec] ;; for whatever dumb reason the Snowflake JDBC driver always returns these as uppercase despite us making them ;; all lower-case - (set (map str/lower-case (sql-jdbc.sync/get-catalogs metadata)))))) + (set (map u/lower-case-en (sql-jdbc.sync/get-catalogs metadata)))))) (let [datasets (atom nil)] (defn- existing-datasets [] diff --git a/resources/sqlite-fixture.db b/resources/sqlite-fixture.db new file mode 100644 index 0000000000000000000000000000000000000000..b390f7cc7d33396a245eb880ba662f8e802683e3 Binary files /dev/null and b/resources/sqlite-fixture.db differ diff --git a/src/metabase/api/field.clj b/src/metabase/api/field.clj index b2178e2dc3375e26061be1e73abbf7c0f1bf0c71..f2fbd191a713267408a0aa9ad069f6fd0cc51c89 100644 --- a/src/metabase/api/field.clj +++ b/src/metabase/api/field.clj @@ -214,7 +214,7 @@ "Update the fields values and human-readable values for a `Field` whose special type is `category`/`city`/`state`/`country` or whose base type is `type/Boolean`. The human-readable values are optional." [id :as {{value-pairs :values} :body}] - {value-pairs [[(s/one s/Num "value") (s/optional su/NonBlankString "human readable value")]]} + {value-pairs [[(s/one (s/maybe s/Num) "value") (s/optional su/NonBlankString "human readable value")]]} (let [field (api/write-check Field id)] (api/check (field-values/field-should-have-field-values? field) [400 (str "You can only update the human readable values of a mapped values of a Field whose value of " diff --git a/src/metabase/api/query_description.clj b/src/metabase/api/query_description.clj index bfebdc99c573efba99e7628249b183a9f50d720b..3d3bc63fad9603380c08bdb15d6af3dc2fa33cc8 100644 --- a/src/metabase/api/query_description.clj +++ b/src/metabase/api/query_description.clj @@ -14,13 +14,21 @@ (defn- get-aggregation-description [metadata query] - (let [field-name (fn [match] (:display_name (Field (mbql.u/field-clause->id-or-literal match))))] + (let [field-name (fn [match] (map + (fn [m] (:display_name (Field (mbql.u/field-clause->id-or-literal m)))) + (mbql.u/match match :field-id)))] (when-let [agg-matches (mbql.u/match query + [:aggregation-options _ (options :guard :display-name)] + {:type :aggregation :arg (:display-name options)} + + [:aggregation-options ag _] + (recur ag) + [:metric arg] {:type :metric - :arg (let [metric (Metric arg)] - (if (not (str/blank? (:name metric))) - (:name metric) - (deferred-tru "[Unknown Metric]")))} + :arg (let [metric (Metric arg)] + (if (not (str/blank? (:name metric))) + (:name metric) + (deferred-tru "[Unknown Metric]")))} [:rows] {:type :rows} [:count] {:type :count} [:cum-count] {:type :cum-count} diff --git a/src/metabase/query_processor/middleware/add_dimension_projections.clj b/src/metabase/query_processor/middleware/add_dimension_projections.clj index 78a8dc1a7f1e3452b694cdba1c73d10431309d44..f949f92e524dfcabeae140922f9ae0239684c2b1 100644 --- a/src/metabase/query_processor/middleware/add_dimension_projections.clj +++ b/src/metabase/query_processor/middleware/add_dimension_projections.clj @@ -155,14 +155,14 @@ "Converts `values` to a type compatible with the base_type found for `col`. These values should be directly comparable with the values returned from the database for the given `col`." [{:keys [base_type] :as col} values] - (map (condp #(isa? %2 %1) base_type - :type/Decimal bigdec - :type/Float double - :type/BigInteger bigint - :type/Integer int - :type/Text str - identity) - values)) + (let [transform (condp #(isa? %2 %1) base_type + :type/Decimal bigdec + :type/Float double + :type/BigInteger bigint + :type/Integer int + :type/Text str + identity)] + (map #(some-> % transform) values))) (def ^:private InternalDimensionInfo {;; index of original column diff --git a/src/metabase/util.clj b/src/metabase/util.clj index 761f2a0200b9045a151c58660426d3c83e51eea4..81e770f1eaa0f67c3a575587b81e549cb5abdcf2 100644 --- a/src/metabase/util.clj +++ b/src/metabase/util.clj @@ -554,15 +554,6 @@ "Increment `n` if it is non-`nil`, otherwise return `1` (e.g. as if incrementing `0`)." (fnil inc 0)) -(defn occurances-of-substring - "Return the number of times SUBSTR occurs in string S." - ^Long [^String s, ^String substr] - (when (and (seq s) (seq substr)) - (loop [index 0, cnt 0] - (if-let [^long new-index (str/index-of s substr index)] - (recur (inc new-index) (inc cnt)) - cnt)))) - (defn select-non-nil-keys "Like `select-keys`, but returns a map only containing keys in KS that are present *and non-nil* in M. @@ -745,6 +736,14 @@ [^CharSequence s] (.. s toString (toLowerCase (Locale/US)))) +(defn upper-case-en + "Locale-agnostic version of `clojure.string/upper-case`. + `clojure.string/upper-case` uses the default locale in conversions, turning + `id` into `İD`, in the Turkish locale. This function always uses the + `Locale/US` locale." + [^CharSequence s] + (.. s toString (toUpperCase (Locale/US)))) + (defn lower-case-map-keys "Changes the keys of a given map to lower case." [m] diff --git a/test/metabase/api/field_test.clj b/test/metabase/api/field_test.clj index b976bcc6669d1caa0af9c664564a1346a66bdfea..a378d3a6c30b4c0d2c0838df7ed6fbb2053b46c3 100644 --- a/test/metabase/api/field_test.clj +++ b/test/metabase/api/field_test.clj @@ -192,18 +192,18 @@ (testing "POST /api/field/:id/values" (testing "Existing field values can be updated (with their human readable values)" (mt/with-temp* [Field [{field-id :id} list-field] - FieldValues [{field-value-id :id} {:values (range 1 5), :field_id field-id}]] + FieldValues [{field-value-id :id} {:values (conj (range 1 5) nil), :field_id field-id}]] (testing "fetch initial values" - (is (= {:values [[1] [2] [3] [4]], :field_id true} + (is (= {:values [[nil] [1] [2] [3] [4]], :field_id true} (mt/boolean-ids-and-timestamps ((mt/user->client :crowberto) :get 200 (format "field/%d/values" field-id)))))) (testing "update values" (is (= {:status "success"} (mt/boolean-ids-and-timestamps ((mt/user->client :crowberto) :post 200 (format "field/%d/values" field-id) - {:values [[1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]]}))))) + {:values [[nil "no $"] [1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]]}))))) (testing "fetch updated values" - (is (= {:values [[1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]], :field_id true} + (is (= {:values [[nil "no $"] [1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]], :field_id true} (mt/boolean-ids-and-timestamps ((mt/user->client :crowberto) :get 200 (format "field/%d/values" field-id)))))))))) diff --git a/test/metabase/api/query_description_test.clj b/test/metabase/api/query_description_test.clj index de5e03d0d4487731885f2a461cc3c7971ead9dd9..ffb603bb1a4a36e7af3ee09fa12ba7efe47f83bf 100644 --- a/test/metabase/api/query_description_test.clj +++ b/test/metabase/api/query_description_test.clj @@ -31,51 +31,61 @@ (testing "with cumulative sum of price" (is (= {:table "Venues" :aggregation [{:type :cum-sum - :arg "Price"}]} + :arg ["Price"]}]} (sut/generate-query-description (Table (mt/id :venues)) (:query (mt/mbql-query :venues {:aggregation [[:cum-sum $price]]})))))) (testing "with equality filter" - (is (= {:table "Venues" + (is (= {:table "Venues" :filter [{:field "Price"}]} (sut/generate-query-description (Table (mt/id :venues)) (:query (mt/mbql-query :venues {:filter [:= [$price 1234]]})))))) (testing "with order-by clause" - (is (= {:table "Venues" + (is (= {:table "Venues" :order-by [{:field "Price" :direction :asc}]} (sut/generate-query-description (Table (mt/id :venues)) (:query (mt/mbql-query :venues {:order-by [[:asc $price]]})))))) (testing "with an aggregation metric" - (tt/with-temp Metric [metric {:table_id (mt/id :venues) :name "Test Metric 1" + (tt/with-temp Metric [metric {:table_id (mt/id :venues) :name "Test Metric 1" :definition {:aggregation [[:count]]}}] - (is (= {:table "Venues" + (is (= {:table "Venues" :aggregation [{:type :metric - :arg "Test Metric 1"}]} + :arg "Test Metric 1"}]} (sut/generate-query-description (Table (mt/id :venues)) (:query (mt/mbql-query :venues - {:aggregation [[:metric (:id metric)]]})))))) + {:aggregation [[:metric (:id metric)]]})))))) - (is (= {:table "Venues" + (is (= {:table "Venues" :aggregation [{:type :metric - :arg (deferred-tru "[Unknown Metric]")}]} + :arg (deferred-tru "[Unknown Metric]")}]} (sut/generate-query-description (Table (mt/id :venues)) (:query (mt/mbql-query :venues {:aggregation [[:metric -1]]})))))) (testing "with segment filters" (tt/with-temp Segment [segment {:name "Test Segment 1"}] - (is (= {:table "Venues" + (is (= {:table "Venues" :filter [{:segment "Test Segment 1"}]} (sut/generate-query-description (Table (mt/id :venues)) (:query (mt/mbql-query :venues {:filter [[:segment (:id segment)]]})))))) - (is (= {:table "Venues" + (is (= {:table "Venues" :filter [{:segment (deferred-tru "[Unknown Segment]")}]} (sut/generate-query-description (Table (mt/id :venues)) (:query (mt/mbql-query :venues - {:filter [[:segment -1]]}))))))))) + {:filter [[:segment -1]]})))))) + + (testing "with named aggregation" + (is (= {:table "Venues" + :aggregation [{:type :aggregation :arg "Nonsensical named metric"}]} + (sut/generate-query-description (Table (mt/id :venues)) + {:aggregation [[:aggregation-options + [:sum [:* + [:field-id (mt/id :venues :latitude)] + [:field-id (mt/id :venues :longitude)]]] + {:display-name "Nonsensical named metric"}]]}))))))) diff --git a/test/metabase/query_processor/middleware/add_dimension_projections_test.clj b/test/metabase/query_processor/middleware/add_dimension_projections_test.clj index 6b069fd9930a815b34f9081eed6d9a1c66951350..05827bbd94136ea04cedfeb01e0249b21ac326e4 100644 --- a/test/metabase/query_processor/middleware/add_dimension_projections_test.clj +++ b/test/metabase/query_processor/middleware/add_dimension_projections_test.clj @@ -143,15 +143,16 @@ (cond-> field (= field-name "CATEGORY_ID") (assoc :dimensions {:type :internal, :name "Foo", :field_id 10} - :values {:human_readable_values ["Foo" "Bar" "Baz" "Qux"] - :values [4 11 29 20]}))))] + :values {:human_readable_values ["Foo" "Bar" "Baz" "Qux" "Quux"] + :values [4 11 29 20 nil]}))))] (is (= {:status :completed - :row_count 5 - :data {:rows [[1 "Red Medicine" 4 3 "Foo"] - [2 "Stout Burgers & Beers" 11 2 "Bar"] - [3 "The Apple Pan" 11 2 "Bar"] - [4 "Wurstküche" 29 2 "Baz"] - [5 "Brite Spot Family Restaurant" 20 2 "Qux"]] + :row_count 6 + :data {:rows [[1 "Red Medicine" 4 3 "Foo"] + [2 "Stout Burgers & Beers" 11 2 "Bar"] + [3 "The Apple Pan" 11 2 "Bar"] + [4 "Wurstküche" 29 2 "Baz"] + [5 "Brite Spot Family Restaurant" 20 2 "Qux"] + [6 "Spaghetti Warehouse" nil 2 "Quux"]] :cols [example-result-cols-id example-result-cols-name (assoc example-result-cols-category-id @@ -166,11 +167,50 @@ example-result-cols-name example-result-cols-category-id example-result-cols-price]} - [[1 "Red Medicine" 4 3] - [2 "Stout Burgers & Beers" 11 2] - [3 "The Apple Pan" 11 2] - [4 "Wurstküche" 29 2] - [5 "Brite Spot Family Restaurant" 20 2]])))))) + [[1 "Red Medicine" 4 3] + [2 "Stout Burgers & Beers" 11 2] + [3 "The Apple Pan" 11 2] + [4 "Wurstküche" 29 2] + [5 "Brite Spot Family Restaurant" 20 2] + [6 "Spaghetti Warehouse" nil 2]])))))) + + (testing "remapping string columns with `human_readable_values`" + ;; swap out `hydrate` with one that will add some fake dimensions and values for CATEGORY_ID. + (with-redefs [hydrate/hydrate (fn [fields & _] + (for [{field-name :name, :as field} fields] + (cond-> field + (= field-name "NAME") + (assoc :dimensions {:type :internal, :name "Foo", :field_id 10} + :values {:human_readable_values ["Appletini" "Bananasplit" "Kiwi-flavored Thing"] + :values ["apple" "banana" "kiwi"]}))))] + (is (= {:status :completed + :row_count 3 + :data {:rows [[1 "apple" 4 3 "Appletini"] + [2 "banana" 11 2 "Bananasplit"] + [3 "kiwi" 11 2 "Kiwi-flavored Thing"]] + :cols [example-result-cols-id + (assoc example-result-cols-name + :remapped_to "Foo") + example-result-cols-category-id + example-result-cols-price + (assoc example-result-cols-foo + :remapped_from "NAME")]}} + (with-redefs [add-dim-projections/add-fk-remaps (fn [query] + [nil query])] + (add-remapping + {} + {:cols [example-result-cols-id + example-result-cols-name + example-result-cols-category-id + example-result-cols-price]} + [[1 "apple" 4 3] + [2 "banana" 11 2] + [3 "kiwi" 11 2]])))))) + + (testing "test that different columns types are transformed" + (is (= (map list [123M 123.0 123N 123 "123"]) + (map #(#'add-dim-projections/transform-values-for-col {:base_type %} [123]) + [:type/Decimal :type/Float :type/BigInteger :type/Integer :type/Text])))) (testing "test that external remappings get the appropriate `:remapped_from`/`:remapped_to` info" (is (= {:status :completed diff --git a/test/metabase/util_test.clj b/test/metabase/util_test.clj index 7e1cd4127cf20c02cb45c8e5de13c585dd98f286..f4e149475edead29d12bfcd77f25c893b0928ad9 100644 --- a/test/metabase/util_test.clj +++ b/test/metabase/util_test.clj @@ -3,8 +3,9 @@ (:require [clojure.test :refer :all] [clojure.tools.macro :as tools.macro] [flatland.ordered.map :refer [ordered-map]] - [metabase.util :as u]) - (:import java.util.Locale)) + [metabase + [test :as mt] + [util :as u]])) (defn- are+-message [expr arglist args] (pr-str @@ -151,25 +152,6 @@ "<<>>" false "{\"a\": 10}" false)) -(deftest occurances-of-substring-test - (testing "should return nil if one or both strings are nil or empty" - (are+ [s substr expected] (= expected - (u/occurances-of-substring s substr)) - nil nil nil - nil "" nil - "" nil nil - "" "" nil - "ABC" "" nil - "" " ABC" nil - ;; non-empty strings - "ABC" "A" 1 - "ABA" "A" 2 - "AAA" "A" 3 - "ABC" "{{id}}" 0 - "WHERE ID = {{id}}" "{{id}}" 1 - "WHERE ID = {{id}} OR USER_ID = {{id}}" "{{id}}" 2 - "WHERE ID = {{id}} OR USER_ID = {{id}} OR TOUCAN_ID = {{id}} OR BIRD_ID = {{bird}}" "{{id}}" 3))) - (deftest select-keys-test (testing "select-non-nil-keys" (is (= {:a 100} @@ -226,14 +208,14 @@ nil nil)) (deftest lower-case-en-test - (let [original-locale (Locale/getDefault)] - (try - (Locale/setDefault (Locale/forLanguageTag "tr")) - ;; `(str/lower-case "ID")` returns "ıd" in the Turkish locale - (is (= "id" - (u/lower-case-en "ID"))) - (finally - (Locale/setDefault original-locale))))) + (mt/with-locale "tr" + (is (= "id" + (u/lower-case-en "ID"))))) + +(deftest upper-case-en-test + (mt/with-locale "tr" + (is (= "ID" + (u/upper-case-en "id"))))) (deftest parse-currency-test (are+ [s expected] (= expected