diff --git a/.circleci/config.yml b/.circleci/config.yml index 3e342413094dacf7bfdac650f7c500568d15410b..429b4358f0b4ba5157df80af73a2204dafa5f31c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -67,6 +67,7 @@ executors: environment: POSTGRES_USER: metabase_test POSTGRES_DB: metabase_test + POSTGRES_HOST_AUTH_METHOD: trust mysql-5-7: parameters: diff --git a/frontend/src/metabase-lib/lib/Dimension.js b/frontend/src/metabase-lib/lib/Dimension.js index 24efb7ac781e7b1709561589e81676042c6fac92..4867920a9efa49d7b3eab2dd1948b8a6944ec09f 100644 --- a/frontend/src/metabase-lib/lib/Dimension.js +++ b/frontend/src/metabase-lib/lib/Dimension.js @@ -847,22 +847,30 @@ export class AggregationDimension extends Dimension { } column(extra = {}) { - const aggregation = this.aggregation(); - const { special_type, ...column } = super.column(); return { - ...column, - // don't pass through `special_type` when aggregating these types - ...(!UNAGGREGATED_SPECIAL_TYPES.has(special_type) && { special_type }), - base_type: aggregation ? aggregation.baseType() : TYPE.Float, + ...super.column(), source: "aggregation", ...extra, }; } field() { - // FIXME: it isn't really correct to return the unaggregated field. return a fake Field object? - const dimension = this.aggregation().dimension(); - return dimension ? dimension.field() : super.field(); + const aggregation = this.aggregation(); + if (!aggregation) { + return super.field(); + } + const dimension = aggregation.dimension(); + const field = dimension && dimension.field(); + const { special_type } = field || {}; + return new Field({ + name: aggregation.columnName(), + display_name: aggregation.displayName(), + base_type: aggregation.baseType(), + // don't pass through `special_type` when aggregating these types + ...(!UNAGGREGATED_SPECIAL_TYPES.has(special_type) && { special_type }), + query: this._query, + metadata: this._metadata, + }); } /** diff --git a/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx b/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx index 06477a1ca35b9f255145390872979c46eaa14491..ad00f82257890b1e59a9617986c119e96ad5be6b 100644 --- a/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx +++ b/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx @@ -1,21 +1,24 @@ import React, { Component } from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; +import _ from "underscore"; +import cx from "classnames"; +import { getIn } from "icepick"; +import { connect } from "react-redux"; import Visualization from "metabase/visualizations/components/Visualization"; import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; import Icon from "metabase/components/Icon"; import Tooltip from "metabase/components/Tooltip"; import CheckBox from "metabase/components/CheckBox"; + import MetabaseAnalytics from "metabase/lib/analytics"; import * as Q_DEPRECATED from "metabase/lib/query"; import { color } from "metabase/lib/colors"; -import { getVisualizationRaw } from "metabase/visualizations"; +import Questions from "metabase/entities/questions"; -import _ from "underscore"; -import cx from "classnames"; -import { getIn } from "icepick"; +import { getVisualizationRaw } from "metabase/visualizations"; function getQueryColumns(card, databases) { const dbId = card.dataset_query.database; @@ -33,6 +36,15 @@ function getQueryColumns(card, databases) { return Q_DEPRECATED.getQueryColumns(table, query); } +// TODO: rework this so we don't have to load all cards up front +@connect( + state => ({ + cards: Questions.selectors.getList(state, { entityQuery: { f: "all" } }), + }), + { + fetchCards: () => Questions.actions.fetchList({ f: "all" }), + }, +) export default class AddSeriesModal extends Component { constructor(props, context) { super(props, context); diff --git a/frontend/src/metabase/dashboard/components/AddToDashSelectQuestionModal.jsx b/frontend/src/metabase/dashboard/components/AddToDashSelectQuestionModal.jsx index 4c7f5ef03e81cb5f478eae7e03febb93a73534eb..5a0a9c00f160006ba703296ae3bee78c076bc9e7 100644 --- a/frontend/src/metabase/dashboard/components/AddToDashSelectQuestionModal.jsx +++ b/frontend/src/metabase/dashboard/components/AddToDashSelectQuestionModal.jsx @@ -1,39 +1,21 @@ -import React, { Component } from "react"; +import React from "react"; import PropTypes from "prop-types"; +import { t } from "ttag"; -import MetabaseAnalytics from "metabase/lib/analytics"; -import AddToDashboard from "metabase/questions/containers/AddToDashboard"; - -export default class AddToDashSelectQuestionModal extends Component { - constructor(props, context) { - super(props, context); +import ModalContent from "metabase/components/ModalContent"; +import QuestionPicker from "metabase/containers/QuestionPicker"; - this.state = { - error: null, - }; - } +import MetabaseAnalytics from "metabase/lib/analytics"; +export default class AddToDashSelectQuestionModal extends React.Component { static propTypes = { dashboard: PropTypes.object.isRequired, - cards: PropTypes.array, - - fetchCards: PropTypes.func.isRequired, addCardToDashboard: PropTypes.func.isRequired, onEditingChange: PropTypes.func.isRequired, - onClose: PropTypes.func.isRequired, }; - async componentDidMount() { - try { - await this.props.fetchCards(); - } catch (error) { - console.error(error); - this.setState({ error }); - } - } - - onAdd = cardId => { + handleAdd = cardId => { this.props.addCardToDashboard({ dashId: this.props.dashboard.id, cardId: cardId, @@ -44,6 +26,13 @@ export default class AddToDashSelectQuestionModal extends Component { }; render() { - return <AddToDashboard onAdd={this.onAdd} onClose={this.props.onClose} />; + return ( + <ModalContent + title={t`Pick a question to add`} + onClose={this.props.onClose} + > + <QuestionPicker onChange={this.handleAdd} /> + </ModalContent> + ); } } diff --git a/frontend/src/metabase/dashboard/components/Dashboard.jsx b/frontend/src/metabase/dashboard/components/Dashboard.jsx index 4b083f1ecd268c680ddd2687544c6e175e2351e3..b9b0b54f3eb7a6e9cf7f0be26714596979efd605 100644 --- a/frontend/src/metabase/dashboard/components/Dashboard.jsx +++ b/frontend/src/metabase/dashboard/components/Dashboard.jsx @@ -24,11 +24,7 @@ import type { QueryParams, } from "metabase/meta/types"; -import type { - Card, - CardId, - VisualizationSettings, -} from "metabase/meta/types/Card"; +import type { CardId, VisualizationSettings } from "metabase/meta/types/Card"; import type { DashboardWithCards, DashboardId, @@ -47,7 +43,6 @@ type Props = { dashboardId: DashboardId, dashboard: DashboardWithCards, - cards: Card[], revisions: { [key: string]: Revision[] }, isAdmin: boolean, @@ -64,7 +59,6 @@ type Props = { addCardToDashboard: ({ dashId: DashCardId, cardId: CardId }) => void, addTextDashCardToDashboard: ({ dashId: DashCardId }) => void, archiveDashboard: (dashboardId: DashboardId) => void, - fetchCards: (filterMode?: string) => void, fetchDashboard: (dashboardId: DashboardId, queryParams: ?QueryParams) => void, saveDashboardAndCards: () => Promise<void>, setDashboardAttributes: ({ [attribute: string]: any }) => void, @@ -133,12 +127,10 @@ export default class Dashboard extends Component { isEditingParameter: PropTypes.bool.isRequired, dashboard: PropTypes.object, - cards: PropTypes.array, parameters: PropTypes.array, addCardToDashboard: PropTypes.func.isRequired, archiveDashboard: PropTypes.func.isRequired, - fetchCards: PropTypes.func.isRequired, fetchDashboard: PropTypes.func.isRequired, saveDashboardAndCards: PropTypes.func.isRequired, setDashboardAttributes: PropTypes.func.isRequired, @@ -181,7 +173,6 @@ export default class Dashboard extends Component { const { addCardOnLoad, fetchDashboard, - fetchCards, addCardToDashboard, setErrorPage, location, @@ -190,8 +181,6 @@ export default class Dashboard extends Component { try { await fetchDashboard(dashboardId, location.query); if (addCardOnLoad != null) { - // we have to load our cards before we can add one - await fetchCards(); this.setEditing(this.props.dashboard); addCardToDashboard({ dashId: dashboardId, cardId: addCardOnLoad }); } diff --git a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx index ecde685a4d41b46a3a029fe7ad55e95469a0b4e7..6a904f05bb7ed91dcdefaf7b062149d1627f04b4 100644 --- a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx @@ -47,7 +47,6 @@ export default class DashboardGrid extends Component { isEditingParameter: PropTypes.bool.isRequired, dashboard: PropTypes.object.isRequired, parameterValues: PropTypes.object.isRequired, - cards: PropTypes.array, setDashCardAttributes: PropTypes.func.isRequired, removeCardFromDashboard: PropTypes.func.isRequired, @@ -161,10 +160,8 @@ export default class DashboardGrid extends Component { <AddSeriesModal dashcard={this.state.addSeriesModalDashCard} dashboard={this.props.dashboard} - cards={this.props.cards} dashcardData={this.props.dashcardData} databases={this.props.databases} - fetchCards={this.props.fetchCards} fetchCardData={this.props.fetchCardData} fetchDatabaseMetadata={this.props.fetchDatabaseMetadata} removeCardFromDashboard={this.props.removeCardFromDashboard} diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader.jsx b/frontend/src/metabase/dashboard/components/DashboardHeader.jsx index dd556a949baac38c8d1dd3320b760d8522d97f32..2e45b80d0ab4ba50f6a4d74ba1f262cc24071b14 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeader.jsx @@ -23,7 +23,7 @@ import MetabaseSettings from "metabase/lib/settings"; import cx from "classnames"; import type { LocationDescriptor, QueryParams } from "metabase/meta/types"; -import type { Card, CardId } from "metabase/meta/types/Card"; +import type { CardId } from "metabase/meta/types/Card"; import type { Parameter, ParameterId, @@ -40,7 +40,6 @@ type Props = { location: LocationDescriptor, dashboard: DashboardWithCards, - cards: Card[], isAdmin: boolean, isEditable: boolean, @@ -56,7 +55,6 @@ type Props = { addCardToDashboard: ({ dashId: DashCardId, cardId: CardId }) => void, addTextDashCardToDashboard: ({ dashId: DashCardId }) => void, archiveDashboard: (dashboardId: DashboardId) => void, - fetchCards: (filterMode?: string) => void, fetchDashboard: (dashboardId: DashboardId, queryParams: ?QueryParams) => void, saveDashboardAndCards: () => Promise<void>, setDashboardAttribute: (attribute: string, value: any) => void, @@ -96,7 +94,6 @@ export default class DashboardHeader extends Component { addCardToDashboard: PropTypes.func.isRequired, addTextDashCardToDashboard: PropTypes.func.isRequired, archiveDashboard: PropTypes.func.isRequired, - fetchCards: PropTypes.func.isRequired, fetchDashboard: PropTypes.func.isRequired, saveDashboardAndCards: PropTypes.func.isRequired, setDashboardAttribute: PropTypes.func.isRequired, @@ -238,8 +235,6 @@ export default class DashboardHeader extends Component { > <AddToDashSelectQuestionModal dashboard={dashboard} - cards={this.props.cards} - fetchCards={this.props.fetchCards} addCardToDashboard={this.props.addCardToDashboard} onEditingChange={this.props.onEditingChange} onClose={() => this.refs.addQuestionModal.toggle()} diff --git a/frontend/src/metabase/dashboard/components/RemoveFromDashboardModal.jsx b/frontend/src/metabase/dashboard/components/RemoveFromDashboardModal.jsx index d8897bef9d4e5a1f19112498326aa3b43bcdcdab..aeb6bd2a88c82702db533dc2bda488f3f49a1377 100644 --- a/frontend/src/metabase/dashboard/components/RemoveFromDashboardModal.jsx +++ b/frontend/src/metabase/dashboard/components/RemoveFromDashboardModal.jsx @@ -7,8 +7,6 @@ import Button from "metabase/components/Button"; import ModalContent from "metabase/components/ModalContent"; export default class RemoveFromDashboardModal extends Component { - state = { deleteCard: false }; - static propTypes = { dashcard: PropTypes.object.isRequired, dashboard: PropTypes.object.isRequired, @@ -21,10 +19,6 @@ export default class RemoveFromDashboardModal extends Component { dashId: this.props.dashboard.id, dashcardId: this.props.dashcard.id, }); - if (this.state.deleteCard) { - // this.props.dispatch(deleteCard(this.props.dashcard.card_id)) - // this.props.dispatch(markCardForDeletion(this.props.dashcard.card_id)) - } this.props.onClose(); MetabaseAnalytics.trackEvent("Dashboard", "Remove Card"); diff --git a/frontend/src/metabase/dashboard/containers/DashboardApp.jsx b/frontend/src/metabase/dashboard/containers/DashboardApp.jsx index 37fbead141f83fd1e094576135a0b19f4e5639b5..2290afaa578f8ed2aa1ea2f67c9bf7f5792e8308 100644 --- a/frontend/src/metabase/dashboard/containers/DashboardApp.jsx +++ b/frontend/src/metabase/dashboard/containers/DashboardApp.jsx @@ -16,7 +16,6 @@ import { getIsEditingParameter, getIsDirty, getDashboardComplete, - getCardList, getCardData, getSlowCards, getEditingParameter, @@ -41,7 +40,6 @@ const mapStateToProps = (state, props) => { isEditingParameter: getIsEditingParameter(state, props), isDirty: getIsDirty(state, props), dashboard: getDashboardComplete(state, props), - cards: getCardList(state, props), dashcardData: getCardData(state, props), slowCards: getSlowCards(state, props), databases: getDatabases(state, props), diff --git a/frontend/src/metabase/dashboard/dashboard.js b/frontend/src/metabase/dashboard/dashboard.js index f9e5b953e6f28d084e1e1196c41179e57503d336..03025bddc8ed0dab11c9e9e1785ec1bf1c42ae32 100644 --- a/frontend/src/metabase/dashboard/dashboard.js +++ b/frontend/src/metabase/dashboard/dashboard.js @@ -2,7 +2,6 @@ import { assoc, dissoc, assocIn, getIn, chain } from "icepick"; import _ from "underscore"; -import moment from "moment"; import { handleActions, @@ -15,6 +14,7 @@ import { defer } from "metabase/lib/promise"; import { normalize, schema } from "normalizr"; import Dashboards from "metabase/entities/dashboards"; +import Questions from "metabase/entities/questions"; import { createParameter, @@ -29,7 +29,7 @@ import type { DashCard, DashCardId, } from "metabase/meta/types/Dashboard"; -import type { Card, CardId } from "metabase/meta/types/Card"; +import type { CardId } from "metabase/meta/types/Card"; import Utils from "metabase/lib/utils"; import { getPositionForNewDashCard } from "metabase/lib/dashboard_grid"; @@ -59,7 +59,6 @@ const DATASET_SLOW_TIMEOUT = 15 * 1000; // normalizr schemas const dashcard = new schema.Entity("dashcard"); -const card = new schema.Entity("card"); const dashboard = new schema.Entity("dashboard", { ordered_cards: [dashcard], }); @@ -70,9 +69,6 @@ export const INITIALIZE = "metabase/dashboard/INITIALIZE"; export const SET_EDITING_DASHBOARD = "metabase/dashboard/SET_EDITING_DASHBOARD"; -export const FETCH_CARDS = "metabase/dashboard/FETCH_CARDS"; -export const DELETE_CARD = "metabase/dashboard/DELETE_CARD"; - // NOTE: this is used in metabase/redux/metadata but can't be imported directly due to circular reference export const FETCH_DASHBOARD = "metabase/dashboard/FETCH_DASHBOARD"; export const SAVE_DASHBOARD_AND_CARDS = @@ -141,58 +137,39 @@ export const markNewCardSeen = createAction(MARK_NEW_CARD_SEEN); export const setDashboardAttributes = createAction(SET_DASHBOARD_ATTRIBUTES); export const setDashCardAttributes = createAction(SET_DASHCARD_ATTRIBUTES); -// TODO: consolidate with questions reducer -export const fetchCards = createThunkAction(FETCH_CARDS, function( - filterMode = "all", -) { - return async function(dispatch, getState) { - const cards = await CardApi.list({ f: filterMode }); - for (const c of cards) { - c.updated_at = moment(c.updated_at); - } - return normalize(cards, [card]); - }; -}); - -export const deleteCard = createThunkAction(DELETE_CARD, function(cardId) { - return async function(dispatch, getState) { - await CardApi.delete({ cardId }); - return cardId; - }; -}); - -export const addCardToDashboard = function({ +export const addCardToDashboard = ({ dashId, cardId, }: { dashId: DashCardId, cardId: CardId, -}) { - return function(dispatch, getState) { - const { dashboards, dashcards, cards } = getState().dashboard; - const dashboard: DashboardWithCards = dashboards[dashId]; - const existingCards: Array<DashCard> = dashboard.ordered_cards - .map(id => dashcards[id]) - .filter(dc => !dc.isRemoved); - const card: Card = cards[cardId]; - const dashcard: DashCard = { - id: Math.random(), // temporary id - dashboard_id: dashId, - card_id: card.id, - card: card, - series: [], - ...getPositionForNewDashCard(existingCards), - parameter_mappings: [], - visualization_settings: {}, - }; - dispatch(createAction(ADD_CARD_TO_DASH)(dashcard)); - dispatch(fetchCardData(card, dashcard, { reload: true, clear: true })); - - // guard in case card was filtered - if (card.dataset_query && card.dataset_query.database) { - dispatch(fetchDatabaseMetadata(card.dataset_query.database)); - } +}) => async (dispatch, getState) => { + await dispatch(Questions.actions.fetch({ id: cardId })); + const card = Questions.selectors.getObject(getState(), { + entityId: cardId, + }); + const { dashboards, dashcards } = getState().dashboard; + const dashboard: DashboardWithCards = dashboards[dashId]; + const existingCards: Array<DashCard> = dashboard.ordered_cards + .map(id => dashcards[id]) + .filter(dc => !dc.isRemoved); + const dashcard: DashCard = { + id: Math.random(), // temporary id + dashboard_id: dashId, + card_id: card.id, + card: card, + series: [], + ...getPositionForNewDashCard(existingCards), + parameter_mappings: [], + visualization_settings: {}, }; + dispatch(createAction(ADD_CARD_TO_DASH)(dashcard)); + dispatch(fetchCardData(card, dashcard, { reload: true, clear: true })); + + // guard in case card was filtered + if (card.dataset_query && card.dataset_query.database) { + dispatch(fetchDatabaseMetadata(card.dataset_query.database)); + } }; export const addDashCardToDashboard = function({ @@ -961,24 +938,6 @@ const isEditing = handleActions( false, ); -// TODO: consolidate with questions reducer -const cards = handleActions( - { - [FETCH_CARDS]: { - next: (state, { payload }) => ({ ...payload.entities.card }), - }, - }, - {}, -); - -const cardList = handleActions( - { - [FETCH_CARDS]: { next: (state, { payload }) => payload.result }, - [DELETE_CARD]: { next: (state, { payload }) => state }, - }, - null, -); - export function syncParametersAndEmbeddingParams(before, after) { if (after.parameters && before.embedding_params) { return Object.keys(before.embedding_params).reduce((memo, embedSlug) => { @@ -1217,8 +1176,6 @@ const loadingDashCards = handleActions( export default combineReducers({ dashboardId, isEditing, - cards, - cardList, dashboards, dashcards, editingParameterId, diff --git a/frontend/src/metabase/dashboard/selectors.js b/frontend/src/metabase/dashboard/selectors.js index edcb7cec0bcf08728bbe72cb93ebf53e42a21175..ba7dddb7b0c792eb0d178c2dd0d5af414e33125c 100644 --- a/frontend/src/metabase/dashboard/selectors.js +++ b/frontend/src/metabase/dashboard/selectors.js @@ -37,12 +37,10 @@ export type MappingsByParameter = { export const getDashboardId = state => state.dashboard.dashboardId; export const getIsEditing = state => state.dashboard.isEditing; -export const getCards = state => state.dashboard.cards; export const getDashboards = state => state.dashboard.dashboards; export const getDashcards = state => state.dashboard.dashcards; export const getCardData = state => state.dashboard.dashcardData; export const getSlowCards = state => state.dashboard.slowCards; -export const getCardIdList = state => state.dashboard.cardList; export const getParameterValues = state => state.dashboard.parameterValues; export const getLoadingStartTime = state => state.dashboard.loadingDashCards.startTime; @@ -80,11 +78,6 @@ export const getIsDirty = createSelector( ), ); -export const getCardList = createSelector( - [getCardIdList, getCards], - (cardIdList, cards) => cardIdList && cardIdList.map(id => cards[id]), -); - export const getEditingParameterId = state => state.dashboard.editingParameterId; @@ -121,10 +114,10 @@ export const getMappingsByParameter = createSelector( } let mappingsByParameter: MappingsByParameter = {}; - const mappings: Array<AugmentedParameterMapping> = []; + const mappings: AugmentedParameterMapping[] = []; const countsByParameter = {}; for (const dashcard of dashboard.ordered_cards) { - const cards: Array<Card> = [dashcard.card].concat(dashcard.series); + const cards: Card[] = [dashcard.card].concat(dashcard.series); for (const mapping: ParameterMapping of dashcard.parameter_mappings || []) { const card = _.findWhere(cards, { id: mapping.card_id }); diff --git a/frontend/src/metabase/lib/formatting.js b/frontend/src/metabase/lib/formatting.js index 129dc62d0a374c39eeda46c603522b18380dabdc..bb5b2b25951ceb0739c3ed18e5ccdc9149effc20 100644 --- a/frontend/src/metabase/lib/formatting.js +++ b/frontend/src/metabase/lib/formatting.js @@ -120,7 +120,7 @@ function getDefaultNumberOptions(options) { return defaults; } -const PRECISION_NUMBER_FORMATTER = d3.format(".2r"); +const PRECISION_NUMBER_FORMATTER = d3.format(".2f"); const FIXED_NUMBER_FORMATTER = d3.format(",.f"); const DECIMAL_DEGREES_FORMATTER = d3.format(".08f"); const DECIMAL_DEGREES_FORMATTER_COMPACT = d3.format(".02f"); @@ -261,23 +261,32 @@ function formatNumberScientific( } } +const DISPLAY_COMPACT_DECIMALS_CUTOFF = 1000; +export const COMPACT_CURRENCY_OPTIONS = { + // Currencies vary in how many decimals they display, so this is probably + // wrong in some cases. Intl.NumberFormat has some of that data built-in, but + // I couldn't figure out how to use it here. + digits: 2, + currency_style: "symbol", +}; + function formatNumberCompact(value: number, options: FormattingOptions) { if (options.number_style === "percent") { return formatNumberCompactWithoutOptions(value * 100) + "%"; } if (options.number_style === "currency") { try { - const { value: currency } = numberFormatterForOptions({ + const nf = numberFormatterForOptions({ ...options, - currency_style: "symbol", - }) - .formatToParts(value) - .find(p => p.type === "currency"); + ...COMPACT_CURRENCY_OPTIONS, + }); - // this special case ensures the "~" comes before the currency - if (value !== 0 && value >= -0.01 && value <= 0.01) { - return `~${currency}0`; + if (Math.abs(value) < DISPLAY_COMPACT_DECIMALS_CUTOFF) { + return nf.format(value); } + const { value: currency } = nf + .formatToParts(value) + .find(p => p.type === "currency"); return currency + formatNumberCompactWithoutOptions(value); } catch (e) { // Intl.NumberFormat failed, so we fall back to a non-currency number @@ -299,10 +308,7 @@ function formatNumberCompactWithoutOptions(value: number) { if (value === 0) { // 0 => 0 return "0"; - } else if (value >= -0.01 && value <= 0.01) { - // 0.01 => ~0 - return "~ 0"; - } else if (value > -1 && value < 1) { + } else if (Math.abs(value) < DISPLAY_COMPACT_DECIMALS_CUTOFF) { // 0.1 => 0.1 return PRECISION_NUMBER_FORMATTER(value).replace(/\.?0+$/, ""); } else { diff --git a/frontend/src/metabase/questions/containers/AddToDashboard.jsx b/frontend/src/metabase/questions/containers/AddToDashboard.jsx deleted file mode 100644 index 117457f3403fcc608f4ed27528a9bdef88160ef9..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/questions/containers/AddToDashboard.jsx +++ /dev/null @@ -1,18 +0,0 @@ -import React, { Component } from "react"; -import { t } from "ttag"; - -import ModalContent from "metabase/components/ModalContent"; -import QuestionPicker from "metabase/containers/QuestionPicker"; - -export default class AddToDashboard extends Component { - render() { - return ( - <ModalContent - title={t`Pick a question to add`} - onClose={this.props.onClose} - > - <QuestionPicker onChange={this.props.onAdd} /> - </ModalContent> - ); - } -} diff --git a/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js b/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js index 8638e8c824ed681402d1f6d5c3b5272b8fba5947..910a357ae1237546cabeb32d4923785eb676abd3 100644 --- a/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js +++ b/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js @@ -5,6 +5,7 @@ import _ from "underscore"; import { color } from "metabase/lib/colors"; import { clipPathReference } from "metabase/lib/dom"; +import { COMPACT_CURRENCY_OPTIONS } from "metabase/lib/formatting"; import { adjustYAxisTicksIfNeeded } from "./apply_axis"; import { isHistogramBar } from "./renderer_utils"; @@ -271,6 +272,31 @@ function onRenderValueLabels(chart, formatYValue, [data]) { return { x, y, showLabelBelow }; }); + const formattingSetting = chart.settings["graph.label_value_formatting"]; + let compact; + if (formattingSetting === "compact") { + compact = true; + } else if (formattingSetting === "full") { + compact = false; + } else { + // for "auto" we use compact if it shortens avg label length by >3 chars + const getAvgLength = compact => { + const options = { + compact, + // We include compact currency options here for both compact and + // non-compact formatting. This prevents auto's logic from depending on + // those settings. + ...COMPACT_CURRENCY_OPTIONS, + // We need this to ensure the settings are used. Otherwise, a cached + // _numberFormatter would take precedence. + _numberFormatter: undefined, + }; + const lengths = data.map(d => formatYValue(d.y, options).length); + return lengths.reduce((sum, l) => sum + l, 0) / lengths.length; + }; + compact = getAvgLength(true) < getAvgLength(false) - 3; + } + // use the chart body so things line up properly const parent = chart.svg().select(".chart-body"); @@ -322,7 +348,7 @@ function onRenderValueLabels(chart, formatYValue, [data]) { .append("text") .attr("class", klass) .attr("text-anchor", "middle") - .text(({ y }) => formatYValue(y, { compact: true })), + .text(({ y }) => formatYValue(y, { compact })), ); }; diff --git a/frontend/src/metabase/visualizations/lib/settings/graph.js b/frontend/src/metabase/visualizations/lib/settings/graph.js index da86c326740777caf0ed4b7bdcc25637ec13db5a..d2f8158098acf1dbfd7bbf278dea71bd20f0243c 100644 --- a/frontend/src/metabase/visualizations/lib/settings/graph.js +++ b/frontend/src/metabase/visualizations/lib/settings/graph.js @@ -346,6 +346,24 @@ export const GRAPH_DISPLAY_VALUES_SETTINGS = { default: "fit", readDependencies: ["graph.show_values"], }, + "graph.label_value_formatting": { + section: t`Display`, + title: t`Value formatting`, + widget: "radio", + getHidden: (series, vizSettings) => + series.length > 1 || + vizSettings["graph.show_values"] !== true || + vizSettings["stackable.stack_type"] === "normalized", + props: { + options: [ + { name: t`Auto`, value: "auto" }, + { name: t`Compact`, value: "compact" }, + { name: t`Full`, value: "full" }, + ], + }, + default: "auto", + readDependencies: ["graph.show_values"], + }, }; export const GRAPH_COLORS_SETTINGS = { diff --git a/frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx b/frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx index 4193395c1c7df8f25c9d43962c71b0363c9e0f0f..fb5d4cccc552dab35bb7d41a38f519da249e5195 100644 --- a/frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx +++ b/frontend/src/metabase/visualizations/visualizations/SmartScalar.jsx @@ -39,7 +39,12 @@ export default class Smart extends React.Component { ], settings, ) => [ - _.find(cols, col => col.name === settings["scalar.field"]) || cols[1], + // try and find a selected field setting + cols.find(col => col.name === settings["scalar.field"]) || + // fall back to the second column + cols[1] || + // but if there's only one column use that + cols[0], ], }), "scalar.switch_positive_negative": { diff --git a/frontend/test/metabase-lib/lib/Dimension.unit.spec.js b/frontend/test/metabase-lib/lib/Dimension.unit.spec.js index ab733bd5fd686bcd9f0037fe4eb65b33a80430b2..dfaa971aaa86ce842c8ad71bc8030fa4f9fde4ef 100644 --- a/frontend/test/metabase-lib/lib/Dimension.unit.spec.js +++ b/frontend/test/metabase-lib/lib/Dimension.unit.spec.js @@ -509,17 +509,21 @@ describe("Dimension", () => { }); }); + function aggregation(agg) { + const query = new StructuredQuery(ORDERS.question(), { + type: "query", + database: SAMPLE_DATASET.id, + query: { + "source-table": ORDERS.id, + aggregation: [agg], + }, + }); + return Dimension.parseMBQL(["aggregation", 0], metadata, query); + } + describe("column()", () => { function sumOf(column) { - const query = new StructuredQuery(ORDERS.question(), { - type: "query", - database: SAMPLE_DATASET.id, - query: { - "source-table": ORDERS.id, - aggregation: [["sum", ["field-id", column.id]]], - }, - }); - return Dimension.parseMBQL(["aggregation", 0], metadata, query); + return aggregation(["sum", ["field-id", column.id]]); } it("should clear unaggregated special types", () => { @@ -534,6 +538,29 @@ describe("Dimension", () => { expect(special_type).toBe("type/Currency"); }); }); + + describe("field()", () => { + it("should return a float field for sum of order total", () => { + const { base_type } = aggregation([ + "sum", + ["field-id", ORDERS.TOTAL.id], + ]).field(); + expect(base_type).toBe("type/Float"); + }); + + it("should return an int field for count distinct of product category", () => { + const { base_type } = aggregation([ + "distinct", + ["field-id", PRODUCTS.CATEGORY.id], + ]).field(); + expect(base_type).toBe("type/Integer"); + }); + + it("should return an int field for count", () => { + const { base_type } = aggregation(["count"]).field(); + expect(base_type).toBe("type/Integer"); + }); + }); }); }); }); diff --git a/frontend/test/metabase/lib/formatting.unit.spec.js b/frontend/test/metabase/lib/formatting.unit.spec.js index 568bbd690a7f8f390e722fc2b0259da5940fa99d..b0e38eee38a0d42019c8060f711850029a6bfb0e 100644 --- a/frontend/test/metabase/lib/formatting.unit.spec.js +++ b/frontend/test/metabase/lib/formatting.unit.spec.js @@ -37,14 +37,14 @@ describe("formatting", () => { it("shouldn't display small numbers as 0", () => { expect(formatNumber(0.1, { compact: true })).toEqual("0.1"); expect(formatNumber(-0.1, { compact: true })).toEqual("-0.1"); - expect(formatNumber(0.01, { compact: true })).toEqual("~ 0"); - expect(formatNumber(-0.01, { compact: true })).toEqual("~ 0"); + expect(formatNumber(0.01, { compact: true })).toEqual("0.01"); + expect(formatNumber(-0.01, { compact: true })).toEqual("-0.01"); }); it("should round up and down", () => { - expect(formatNumber(1.01, { compact: true })).toEqual("1"); - expect(formatNumber(-1.01, { compact: true })).toEqual("-1"); - expect(formatNumber(1.9, { compact: true })).toEqual("2"); - expect(formatNumber(-1.9, { compact: true })).toEqual("-2"); + expect(formatNumber(1.01, { compact: true })).toEqual("1.01"); + expect(formatNumber(-1.01, { compact: true })).toEqual("-1.01"); + expect(formatNumber(1.9, { compact: true })).toEqual("1.9"); + expect(formatNumber(-1.9, { compact: true })).toEqual("-1.9"); }); it("should format large numbers with metric units", () => { expect(formatNumber(1, { compact: true })).toEqual("1"); @@ -55,12 +55,12 @@ describe("formatting", () => { const options = { compact: true, number_style: "percent" }; expect(formatNumber(0, options)).toEqual("0%"); expect(formatNumber(0.001, options)).toEqual("0.1%"); - expect(formatNumber(0.0001, options)).toEqual("~ 0%"); + expect(formatNumber(0.0001, options)).toEqual("0.01%"); expect(formatNumber(0.001234, options)).toEqual("0.12%"); expect(formatNumber(0.1, options)).toEqual("10%"); - expect(formatNumber(0.1234, options)).toEqual("12%"); - expect(formatNumber(0.019, options)).toEqual("2%"); - expect(formatNumber(0.021, options)).toEqual("2%"); + expect(formatNumber(0.1234, options)).toEqual("12.34%"); + expect(formatNumber(0.019, options)).toEqual("1.9%"); + expect(formatNumber(0.021, options)).toEqual("2.1%"); expect(formatNumber(11.11, options)).toEqual("1.1k%"); expect(formatNumber(-0.22, options)).toEqual("-22%"); }); @@ -79,9 +79,11 @@ describe("formatting", () => { number_style: "currency", currency: "USD", }; - expect(formatNumber(0, options)).toEqual("$0"); - expect(formatNumber(0.001, options)).toEqual("~$0"); - expect(formatNumber(7.24, options)).toEqual("$7"); + expect(formatNumber(0, options)).toEqual("$0.00"); + expect(formatNumber(0.001, options)).toEqual("$0.00"); + expect(formatNumber(7.24, options)).toEqual("$7.24"); + expect(formatNumber(7.249, options)).toEqual("$7.25"); + expect(formatNumber(724.9, options)).toEqual("$724.90"); expect(formatNumber(1234.56, options)).toEqual("$1.2k"); expect(formatNumber(1234567.89, options)).toEqual("$1.2M"); expect(formatNumber(-1234567.89, options)).toEqual("$-1.2M"); diff --git a/frontend/test/metabase/visualizations/components/SmartScalar.unit.spec.js b/frontend/test/metabase/visualizations/components/SmartScalar.unit.spec.js index c6f132bd6f7b5bd5484688c55ba8b75adf2a4d49..54275f0b998a7e5b66b060f17199bd745e4da013 100644 --- a/frontend/test/metabase/visualizations/components/SmartScalar.unit.spec.js +++ b/frontend/test/metabase/visualizations/components/SmartScalar.unit.spec.js @@ -4,6 +4,7 @@ import { render, cleanup } from "@testing-library/react"; import { NumberColumn, DateTimeColumn } from "../__support__/visualizations"; import Visualization from "metabase/visualizations/components/Visualization"; +import { getSettingsWidgetsForSeries } from "metabase/visualizations/lib/settings/visualization"; const series = ({ rows, insights }) => { const cols = [ @@ -93,4 +94,10 @@ describe("SmartScalar", () => { ); getAllByText("8,000%"); }); + + it("shouldn't throw an error getting settings for single-column data", () => { + const card = { display: "smartscalar", visualization_settings: {} }; + const data = { cols: [NumberColumn({ name: "Count" })], rows: [[100]] }; + expect(() => getSettingsWidgetsForSeries([{ card, data }])).not.toThrow(); + }); });