From aec0318c3bff86633d9f60f76b3877ad5b4d486c Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Tue, 22 Oct 2024 12:02:13 +0300 Subject: [PATCH] Dashboard: show comparison for breakdowns (#4692) * Comparisons: Move code to LegacyQueryBuilder * WIP: Return comparison results to frontend * refactor: remove useless param * Different result format * Pass object to metric.renderValue * remove dead code * Fixup response format * Comparison in a tooltip Not perfect at all, but a good start. Problems arise with money etc. * Simple change arrow * Extract metric entry to ts * popper attempt WIP * Slightly nicer content * Solve warning * Unified changeArrow in app * Remove needless spanning * Always set `graph_metric` in top stats. FE already has business logic around whether a given metric is graphable * Remove dead code * Move Money module under dashboard utils, keep in build * change definition to take in a `formatter` and store default formatters in another (typed) const * Use standard system for formatting numbers * Arrows only in table * remove dead import * Inline renderValue * Render metric name in tooltip * numberFormatter -> numberShortFormatter * numberShortFormatter update * Separate long/short formatters * Use long vs short formatters * Put column name into tooltip * Slightly improved label handling for percentages, conversion rate * Improved boundary handling in tooltip.js * Iterate tooltips, no tooltip for - revenue * Update top stats tests after graph_metric change * Change revenue metrics stats API return structure Conversion now happens earlier in query pipeline, we return float for comparison purposes * useQueryContext in a component * graph_metric for current visitors to fix realtime view * No tooltips if fully - row * renderValue as a proper function * Simplify MetricEntry * Use common const * tooltip to typescript * More explicit return structure * metric-entry -> metric-value * Restore some files * ChangeArrow * Restore MoreLink * Fix typing in MoreLink * * Tests for MetricValue and ChangeArrow * details modal fixups * re-add space between arrow and percentage * Solve stylelint issues * Update test * Format * Add flag `breakdown_comparisons_ui` * reformat * Remove no change icon, better alignment * Revert "Remove no change icon, better alignment" This reverts commit a8d62b6383158e198d726a9524f703ef4b9faed6. * number-formatter.ts * numberLongFormatter refactor * useMemo dependency * Handle nulls/undefined in top stats --------- Co-authored-by: Uku Taht --- assets/js/dashboard/components/table.tsx | 6 +- assets/js/dashboard/extra/funnel.js | 6 +- assets/js/dashboard/extra/money.js | 9 - assets/js/dashboard/site-context.tsx | 2 +- .../js/dashboard/stats/graph/graph-tooltip.js | 5 +- assets/js/dashboard/stats/graph/graph-util.js | 15 -- assets/js/dashboard/stats/graph/line-graph.js | 5 +- assets/js/dashboard/stats/graph/top-stats.js | 99 +++----- assets/js/dashboard/stats/locations/map.tsx | 4 +- .../stats/modals/google-keywords.tsx | 13 +- .../stats/reports/change-arrow.test.tsx | 60 +++++ .../dashboard/stats/reports/change-arrow.tsx | 49 ++++ assets/js/dashboard/stats/reports/list.js | 164 +++++++++----- .../stats/reports/metric-formatter.ts | 69 ++++++ .../stats/reports/metric-value.test.tsx | 212 ++++++++++++++++++ .../dashboard/stats/reports/metric-value.tsx | 125 +++++++++++ assets/js/dashboard/stats/reports/metrics.js | 194 ++++++++++------ .../dashboard/stats/sources/search-terms.js | 4 +- assets/js/dashboard/util/money.ts | 17 ++ .../dashboard/util/number-formatter.test.ts | 33 +++ ...umber-formatter.js => number-formatter.ts} | 16 +- .../util/{tooltip.js => tooltip.tsx} | 19 +- extra/lib/plausible/stats/goal/revenue.ex | 8 +- lib/plausible/stats/breakdown.ex | 24 +- lib/plausible/stats/compare.ex | 8 +- .../stats/legacy/legacy_query_builder.ex | 33 +++ .../controllers/api/stats_controller.ex | 110 ++------- .../controllers/stats_controller.ex | 5 +- .../api/stats_controller/conversions_test.exs | 16 +- .../custom_prop_breakdown_test.exs | 16 +- .../api/stats_controller/top_stats_test.exs | 79 ++++--- 31 files changed, 1034 insertions(+), 391 deletions(-) delete mode 100644 assets/js/dashboard/extra/money.js create mode 100644 assets/js/dashboard/stats/reports/change-arrow.test.tsx create mode 100644 assets/js/dashboard/stats/reports/change-arrow.tsx create mode 100644 assets/js/dashboard/stats/reports/metric-formatter.ts create mode 100644 assets/js/dashboard/stats/reports/metric-value.test.tsx create mode 100644 assets/js/dashboard/stats/reports/metric-value.tsx create mode 100644 assets/js/dashboard/util/money.ts create mode 100644 assets/js/dashboard/util/number-formatter.test.ts rename assets/js/dashboard/util/{number-formatter.js => number-formatter.ts} (78%) rename assets/js/dashboard/util/{tooltip.js => tooltip.tsx} (71%) diff --git a/assets/js/dashboard/components/table.tsx b/assets/js/dashboard/components/table.tsx index 3e6eeee37..53976ec02 100644 --- a/assets/js/dashboard/components/table.tsx +++ b/assets/js/dashboard/components/table.tsx @@ -9,7 +9,7 @@ export type ColumnConfiguraton> = { /** Unique column ID, used for sorting purposes and to get the value of the cell using rowItem[key] */ key: keyof T /** Column title */ - label: ReactNode + label: string /** If defined, the column is considered sortable. @see SortButton */ onSort?: () => void sortDirection?: SortDirection @@ -20,7 +20,7 @@ export type ColumnConfiguraton> = { /** * Function used to transform the value found at item[key] for the cell. Superseded by renderItem if present. @example 1120 => "1.1k" */ - renderValue?: (value: unknown) => ReactNode + renderValue?: (item: T) => ReactNode /** Function used to create richer cells */ renderItem?: (item: T) => ReactNode } @@ -85,7 +85,7 @@ export const ItemRow = >({ {renderItem ? renderItem(item) : renderValue - ? renderValue(item[key]) + ? renderValue(item) : (item[key] ?? '')} ))} diff --git a/assets/js/dashboard/extra/funnel.js b/assets/js/dashboard/extra/funnel.js index 477b110d4..c64da5c2e 100644 --- a/assets/js/dashboard/extra/funnel.js +++ b/assets/js/dashboard/extra/funnel.js @@ -3,7 +3,7 @@ import FlipMove from 'react-flip-move'; import Chart from 'chart.js/auto'; import FunnelTooltip from './funnel-tooltip'; import ChartDataLabels from 'chartjs-plugin-datalabels'; -import numberFormatter from '../util/number-formatter'; +import { numberShortFormatter } from '../util/number-formatter'; import Bar from '../stats/bar'; import RocketIcon from '../stats/modals/rocket-icon'; @@ -103,7 +103,7 @@ export default function Funnel({ funnelName, tabs }) { const formatDataLabel = (visitors, ctx) => { if (ctx.dataset.label === 'Visitors') { const conversionRate = funnel.steps[ctx.dataIndex].conversion_rate - return `${conversionRate}% \n(${numberFormatter(visitors)} Visitors)` + return `${conversionRate}% \n(${numberShortFormatter(visitors)} Visitors)` } else { return null } @@ -330,7 +330,7 @@ export default function Funnel({ funnelName, tabs }) { - {numberFormatter(step.visitors)} + {numberShortFormatter(step.visitors)} diff --git a/assets/js/dashboard/extra/money.js b/assets/js/dashboard/extra/money.js deleted file mode 100644 index 785ba403c..000000000 --- a/assets/js/dashboard/extra/money.js +++ /dev/null @@ -1,9 +0,0 @@ -import React from 'react' - -export default function Money({ formatted }) { - if (formatted) { - return {formatted.short} - } else { - return "-" - } -} diff --git a/assets/js/dashboard/site-context.tsx b/assets/js/dashboard/site-context.tsx index 9c37e19c5..863745cec 100644 --- a/assets/js/dashboard/site-context.tsx +++ b/assets/js/dashboard/site-context.tsx @@ -45,7 +45,7 @@ const siteContextDefaultValue = { embedded: false, background: undefined as string | undefined, isDbip: false, - flags: {}, + flags: {} as { breakdown_comparisons_ui?: boolean }, validIntervalsByPeriod: {} as Record>, shared: false } diff --git a/assets/js/dashboard/stats/graph/graph-tooltip.js b/assets/js/dashboard/stats/graph/graph-tooltip.js index f6536d2a4..fffeec828 100644 --- a/assets/js/dashboard/stats/graph/graph-tooltip.js +++ b/assets/js/dashboard/stats/graph/graph-tooltip.js @@ -1,5 +1,6 @@ -import { METRIC_FORMATTER, METRIC_LABELS } from './graph-util' import dateFormatter from './date-formatter' +import { METRIC_LABELS } from './graph-util' +import { MetricFormatterShort } from '../reports/metric-formatter' const renderBucketLabel = function(query, graphData, label, comparison = false) { let isPeriodFull = graphData.full_intervals?.[label] @@ -44,7 +45,7 @@ const buildTooltipData = function(query, graphData, metric, tooltipModel) { const comparisonValue = comparisonData?.raw || 0 const comparisonDifference = label && comparisonLabel && calculatePercentageDifference(comparisonValue, value) - const metricFormatter = METRIC_FORMATTER[metric] + const metricFormatter = MetricFormatterShort[metric] const formattedValue = metricFormatter(value) const formattedComparisonValue = comparisonData && metricFormatter(comparisonValue) diff --git a/assets/js/dashboard/stats/graph/graph-util.js b/assets/js/dashboard/stats/graph/graph-util.js index 4b4cc0be4..f4f520fd3 100644 --- a/assets/js/dashboard/stats/graph/graph-util.js +++ b/assets/js/dashboard/stats/graph/graph-util.js @@ -1,4 +1,3 @@ -import numberFormatter, {durationFormatter} from '../../util/number-formatter' import { getFiltersByKeyPrefix, hasGoalFilter } from '../../util/filters' import { revenueAvailable } from '../../query' @@ -36,20 +35,6 @@ export const METRIC_LABELS = { 'total_revenue': 'Total Revenue', } -export const METRIC_FORMATTER = { - 'visitors': numberFormatter, - 'pageviews': numberFormatter, - 'events': numberFormatter, - 'visits': numberFormatter, - 'views_per_visit': (number) => (number), - 'bounce_rate': (number) => (`${number}%`), - 'visit_duration': durationFormatter, - 'conversions': numberFormatter, - 'conversion_rate': (number) => (`${number}%`), - 'total_revenue': numberFormatter, - 'average_revenue': numberFormatter, -} - const buildComparisonDataset = function(comparisonPlot) { if (!comparisonPlot) return [] diff --git a/assets/js/dashboard/stats/graph/line-graph.js b/assets/js/dashboard/stats/graph/line-graph.js index 2b0883251..6c0b1e3a9 100644 --- a/assets/js/dashboard/stats/graph/line-graph.js +++ b/assets/js/dashboard/stats/graph/line-graph.js @@ -3,11 +3,12 @@ import { useAppNavigate } from '../../navigation/use-app-navigate'; import { useQueryContext } from '../../query-context'; import Chart from 'chart.js/auto'; import GraphTooltip from './graph-tooltip' -import { buildDataSet, METRIC_LABELS, METRIC_FORMATTER } from './graph-util' +import { buildDataSet, METRIC_LABELS } from './graph-util' import dateFormatter from './date-formatter'; import FadeIn from '../../fade-in'; import classNames from 'classnames'; import { hasGoalFilter } from '../../util/filters'; +import { MetricFormatterShort } from '../reports/metric-formatter' const calculateMaximumY = function(dataset) { const yAxisValues = dataset @@ -76,7 +77,7 @@ class LineGraph extends React.Component { min: 0, suggestedMax: calculateMaximumY(dataSet), ticks: { - callback: METRIC_FORMATTER[metric], + callback: MetricFormatterShort[metric], color: this.props.darkTheme ? 'rgb(243, 244, 246)' : undefined }, grid: { diff --git a/assets/js/dashboard/stats/graph/top-stats.js b/assets/js/dashboard/stats/graph/top-stats.js index 24d544b37..4bdd477ed 100644 --- a/assets/js/dashboard/stats/graph/top-stats.js +++ b/assets/js/dashboard/stats/graph/top-stats.js @@ -4,13 +4,17 @@ import React from 'react' import { Tooltip } from '../../util/tooltip' import { SecondsSinceLastLoad } from '../../util/seconds-since-last-load' import classNames from 'classnames' -import numberFormatter, { durationFormatter } from '../../util/number-formatter' import * as storage from '../../util/storage' import { formatDateRange } from '../../util/date' import { getGraphableMetrics } from './graph-util' import { useQueryContext } from '../../query-context' import { useSiteContext } from '../../site-context' import { useLastLoadContext } from '../../last-load-context' +import { ChangeArrow } from '../reports/change-arrow' +import { + MetricFormatterShort, + MetricFormatterLong +} from '../reports/metric-formatter' function Maybe({ condition, children }) { if (condition) { @@ -20,67 +24,21 @@ function Maybe({ condition, children }) { } } -function renderPercentageComparison(name, comparison, forceDarkBg = false) { - const formattedComparison = numberFormatter(Math.abs(comparison)) - - const defaultClassName = classNames({ - 'pl-2 text-xs dark:text-gray-100': !forceDarkBg, - 'pl-2 text-xs text-gray-100': forceDarkBg - }) - - const noChangeClassName = classNames({ - 'pl-2 text-xs text-gray-700 dark:text-gray-300': !forceDarkBg, - 'pl-2 text-xs text-gray-300': forceDarkBg - }) - - if (comparison > 0) { - const color = name === 'Bounce rate' ? 'text-red-400' : 'text-green-500' - return ( - - {' '} - {formattedComparison}% - - ) - } else if (comparison < 0) { - const color = name === 'Bounce rate' ? 'text-green-500' : 'text-red-400' - return ( - - {' '} - {formattedComparison}% - - ) - } else if (comparison === 0) { - return 〰 0% +function topStatNumberShort(metric, value) { + if (typeof value == 'number') { + const formatter = MetricFormatterShort[metric] + return formatter(value) } else { return null } } -function topStatNumberShort(name, value) { - if (['visit duration', 'time on page'].includes(name.toLowerCase())) { - return durationFormatter(value) - } else if (['bounce rate', 'conversion rate'].includes(name.toLowerCase())) { - return value + '%' - } else if ( - ['average revenue', 'total revenue'].includes(name.toLowerCase()) - ) { - return value?.short +function topStatNumberLong(metric, value) { + if (typeof value == 'number') { + const formatter = MetricFormatterLong[metric] + return formatter(value) } else { - return numberFormatter(value) - } -} - -function topStatNumberLong(name, value) { - if (['visit duration', 'time on page'].includes(name.toLowerCase())) { - return durationFormatter(value) - } else if (['bounce rate', 'conversion rate'].includes(name.toLowerCase())) { - return value + '%' - } else if ( - ['average revenue', 'total revenue'].includes(name.toLowerCase()) - ) { - return value?.long - } else { - return (value || 0).toLocaleString() + return null } } @@ -97,17 +55,20 @@ export default function TopStats({ data, onMetricUpdate, tooltipBoundary }) {
{query.comparison && (
- {topStatNumberLong(stat.name, stat.value)} vs.{' '} - {topStatNumberLong(stat.name, stat.comparison_value)} {statName} - - {renderPercentageComparison(stat.name, stat.change, true)} - + {topStatNumberLong(stat.graph_metric, stat.value)} vs.{' '} + {topStatNumberLong(stat.graph_metric, stat.comparison_value)}{' '} + {statName} +
)} {!query.comparison && (
- {topStatNumberLong(stat.name, stat.value)} {statName} + {topStatNumberLong(stat.graph_metric, stat.value)} {statName}
)} @@ -123,7 +84,7 @@ export default function TopStats({ data, onMetricUpdate, tooltipBoundary }) { function canMetricBeGraphed(stat) { const graphableMetrics = getGraphableMetrics(query, site) - return stat.graph_metric && graphableMetrics.includes(stat.graph_metric) + return graphableMetrics.includes(stat.graph_metric) } function maybeUpdateMetric(stat) { @@ -200,10 +161,14 @@ export default function TopStats({ data, onMetricUpdate, tooltipBoundary }) { className="font-bold text-xl dark:text-gray-100" id={stat.graph_metric} > - {topStatNumberShort(stat.name, stat.value)} + {topStatNumberShort(stat.graph_metric, stat.value)}

- - {renderPercentageComparison(stat.name, stat.change)} + + @@ -216,7 +181,7 @@ export default function TopStats({ data, onMetricUpdate, tooltipBoundary }) {

- {topStatNumberShort(stat.name, stat.comparison_value)} + {topStatNumberShort(stat.graph_metric, stat.comparison_value)}

{formatDateRange(site, data.comparing_from, data.comparing_to)} diff --git a/assets/js/dashboard/stats/locations/map.tsx b/assets/js/dashboard/stats/locations/map.tsx index 76e80baac..de2b07c37 100644 --- a/assets/js/dashboard/stats/locations/map.tsx +++ b/assets/js/dashboard/stats/locations/map.tsx @@ -5,7 +5,7 @@ import classNames from 'classnames' import * as api from '../../api' import { replaceFilterByPrefix, cleanLabels } from '../../util/filters' import { useAppNavigate } from '../../navigation/use-app-navigate' -import numberFormatter from '../../util/number-formatter' +import { numberShortFormatter } from '../../util/number-formatter' import * as topojson from 'topojson-client' import { useQuery } from '@tanstack/react-query' import { useSiteContext } from '../../site-context' @@ -165,7 +165,7 @@ const WorldMap = ({ x={tooltip.x} y={tooltip.y} name={hoveredCountryData.name} - value={numberFormatter(hoveredCountryData.visitors)} + value={numberShortFormatter(hoveredCountryData.visitors)} label={ labels[hoveredCountryData.visitors === 1 ? 'singular' : 'plural'] } diff --git a/assets/js/dashboard/stats/modals/google-keywords.tsx b/assets/js/dashboard/stats/modals/google-keywords.tsx index 5ee7bddfd..f3f0ddeca 100644 --- a/assets/js/dashboard/stats/modals/google-keywords.tsx +++ b/assets/js/dashboard/stats/modals/google-keywords.tsx @@ -6,12 +6,9 @@ import Modal from './modal' import { useQueryContext } from '../../query-context' import { useSiteContext } from '../../site-context' import { usePaginatedGetAPI } from '../../hooks/api-client' +import { createVisitors, Metric } from '../reports/metrics' import { - createVisitors, - Metric, - renderNumberWithTooltip -} from '../reports/metrics' -import numberFormatter, { + numberShortFormatter, percentageFormatter } from '../../util/number-formatter' import { apiPath } from '../../util/url' @@ -33,21 +30,21 @@ const metrics = [ width: 'w-28', key: 'impressions', renderLabel: () => 'Impressions', - renderValue: renderNumberWithTooltip, + formatter: numberShortFormatter, sortable: false }), new Metric({ width: 'w-16', key: 'ctr', renderLabel: () => 'CTR', - renderValue: percentageFormatter, + formatter: percentageFormatter, sortable: false }), new Metric({ width: 'w-28', key: 'position', renderLabel: () => 'Position', - renderValue: numberFormatter, + formatter: numberShortFormatter, sortable: false }) ] diff --git a/assets/js/dashboard/stats/reports/change-arrow.test.tsx b/assets/js/dashboard/stats/reports/change-arrow.test.tsx new file mode 100644 index 000000000..eff8b7239 --- /dev/null +++ b/assets/js/dashboard/stats/reports/change-arrow.test.tsx @@ -0,0 +1,60 @@ +/** @format */ + +import React from 'react' +import { render, screen } from '@testing-library/react' +import { ChangeArrow } from './change-arrow' + +it('renders green for positive change', () => { + render() + + const arrowElement = screen.getByTestId('change-arrow') + + expect(arrowElement).toHaveTextContent('↑ 1%') + expect(arrowElement.children[0]).toHaveClass('text-green-500') +}) + +it('renders red for positive change', () => { + render() + + const arrowElement = screen.getByTestId('change-arrow') + + expect(arrowElement).toHaveTextContent('↓ 10%') + expect(arrowElement.children[0]).toHaveClass('text-red-400') +}) + +it('renders tilde for no change', () => { + render() + + const arrowElement = screen.getByTestId('change-arrow') + + expect(arrowElement).toHaveTextContent('〰 0%') +}) + +it('inverts colors for positive bounce_rate change', () => { + render() + + const arrowElement = screen.getByTestId('change-arrow') + + expect(arrowElement).toHaveTextContent('↑ 15%') + expect(arrowElement.children[0]).toHaveClass('text-red-400') +}) + +it('inverts colors for negative bounce_rate change', () => { + render() + + const arrowElement = screen.getByTestId('change-arrow') + + expect(arrowElement).toHaveTextContent('↓ 3%') + expect(arrowElement.children[0]).toHaveClass('text-green-500') +}) + +it('renders with text hidden', () => { + render( + + ) + + const arrowElement = screen.getByTestId('change-arrow') + + expect(arrowElement).toHaveTextContent('↓') + expect(arrowElement.children[0]).toHaveClass('text-red-400') +}) diff --git a/assets/js/dashboard/stats/reports/change-arrow.tsx b/assets/js/dashboard/stats/reports/change-arrow.tsx new file mode 100644 index 000000000..3f77e1055 --- /dev/null +++ b/assets/js/dashboard/stats/reports/change-arrow.tsx @@ -0,0 +1,49 @@ +/** @format */ + +import React from 'react' +import { Metric } from '../../../types/query-api' +import { numberShortFormatter } from '../../util/number-formatter' + +export function ChangeArrow({ + change, + metric, + className, + hideNumber +}: { + change: number + metric: Metric + className: string + hideNumber?: boolean +}) { + const formattedChange = hideNumber + ? null + : ` ${numberShortFormatter(Math.abs(change))}%` + + let content = null + + if (change > 0) { + const color = metric === 'bounce_rate' ? 'text-red-400' : 'text-green-500' + content = ( + <> + + {formattedChange} + + ) + } else if (change < 0) { + const color = metric === 'bounce_rate' ? 'text-green-500' : 'text-red-400' + content = ( + <> + + {formattedChange} + + ) + } else if (change === 0) { + content = <>〰{formattedChange} + } + + return ( + + {content} + + ) +} diff --git a/assets/js/dashboard/stats/reports/list.js b/assets/js/dashboard/stats/reports/list.js index 7f1eb3fd8..48474f459 100644 --- a/assets/js/dashboard/stats/reports/list.js +++ b/assets/js/dashboard/stats/reports/list.js @@ -1,26 +1,43 @@ -import React, { useState, useEffect, useCallback } from 'react'; -import { AppNavigationLink } from '../../navigation/use-app-navigate'; -import FlipMove from 'react-flip-move'; +/** @format */ -import FadeIn from '../../fade-in'; -import MoreLink from '../more-link'; -import Bar from '../bar'; -import LazyLoader from '../../components/lazy-loader'; -import classNames from 'classnames'; -import { trimURL } from '../../util/url'; -import { cleanLabels, replaceFilterByPrefix, isRealTimeDashboard, hasGoalFilter, plainFilterText } from '../../util/filters'; -import { useQueryContext } from '../../query-context'; +import React, { useState, useEffect, useCallback } from 'react' +import { AppNavigationLink } from '../../navigation/use-app-navigate' +import FlipMove from 'react-flip-move' + +import FadeIn from '../../fade-in' +import MoreLink from '../more-link' +import Bar from '../bar' +import LazyLoader from '../../components/lazy-loader' +import classNames from 'classnames' +import { trimURL } from '../../util/url' +import { + cleanLabels, + replaceFilterByPrefix, + isRealTimeDashboard, + hasGoalFilter, + plainFilterText +} from '../../util/filters' +import { useQueryContext } from '../../query-context' const MAX_ITEMS = 9 export const MIN_HEIGHT = 380 const ROW_HEIGHT = 32 const ROW_GAP_HEIGHT = 4 -const DATA_CONTAINER_HEIGHT = (ROW_HEIGHT + ROW_GAP_HEIGHT) * (MAX_ITEMS - 1) + ROW_HEIGHT +const DATA_CONTAINER_HEIGHT = + (ROW_HEIGHT + ROW_GAP_HEIGHT) * (MAX_ITEMS - 1) + ROW_HEIGHT const COL_MIN_WIDTH = 70 -export function FilterLink({ path, filterInfo, onClick, children, extraClass }) { - const { query } = useQueryContext(); - const className = classNames(`${extraClass}`, { 'hover:underline': !!filterInfo }) +export function FilterLink({ + path, + filterInfo, + onClick, + children, + extraClass +}) { + const { query } = useQueryContext() + const className = classNames(`${extraClass}`, { + 'hover:underline': !!filterInfo + }) if (filterInfo) { const { prefix, filter, labels } = filterInfo @@ -33,7 +50,11 @@ export function FilterLink({ path, filterInfo, onClick, children, extraClass }) className={className} path={path} onClick={onClick} - search={(search) => ({...search, filters: newFilters, labels: newLabels})} + search={(search) => ({ + ...search, + filters: newFilters, + labels: newLabels + })} > {children} @@ -53,7 +74,14 @@ function ExternalLink({ item, externalLinkDest }) { href={dest} className="w-4 h-4 hidden group-hover:block" > - + + + + ) } @@ -105,30 +133,44 @@ function ExternalLink({ item, externalLinkDest }) { * | LISTITEM_1.name | LISTITEM_1[METRIC_1.key] | LISTITEM_1[METRIC_2.key] | ... * | LISTITEM_2.name | LISTITEM_2[METRIC_1.key] | LISTITEM_2[METRIC_2.key] | ... */ -export default function ListReport({ keyLabel, metrics, colMinWidth = COL_MIN_WIDTH, afterFetchData, detailsLinkProps, maybeHideDetails, onClick, color, getFilterFor, renderIcon, externalLinkDest, fetchData }) { - const { query } = useQueryContext(); +export default function ListReport({ + keyLabel, + metrics, + colMinWidth = COL_MIN_WIDTH, + afterFetchData, + detailsLinkProps, + maybeHideDetails, + onClick, + color, + getFilterFor, + renderIcon, + externalLinkDest, + fetchData +}) { + const { query } = useQueryContext() const [state, setState] = useState({ loading: true, list: null }) const [visible, setVisible] = useState(false) - const isRealtime = isRealTimeDashboard(query); - const goalFilterApplied = hasGoalFilter(query); + const isRealtime = isRealTimeDashboard(query) + const goalFilterApplied = hasGoalFilter(query) const getData = useCallback(() => { if (!isRealtime) { setState({ loading: true, list: null }) } - fetchData() - .then((response) => { - if (afterFetchData) { - afterFetchData(response) - } + fetchData().then((response) => { + if (afterFetchData) { + afterFetchData(response) + } - setState({ loading: false, list: response.results }) - }) - // eslint-disable-next-line react-hooks/exhaustive-deps + setState({ loading: false, list: response.results }) + }) + // eslint-disable-next-line react-hooks/exhaustive-deps }, [keyLabel, query]) - const onVisible = () => { setVisible(true) } + const onVisible = () => { + setVisible(true) + } useEffect(() => { if (isRealtime) { @@ -137,18 +179,22 @@ export default function ListReport({ keyLabel, metrics, colMinWidth = COL_MIN_WI // only read the new metrics once the new list is loaded. setState({ loading: true, list: null }) } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [goalFilterApplied]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [goalFilterApplied]) useEffect(() => { if (visible) { - if (isRealtime) { document.addEventListener('tick', getData) } + if (isRealtime) { + document.addEventListener('tick', getData) + } getData() } - return () => { document.removeEventListener('tick', getData) } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [keyLabel, query, visible]); + return () => { + document.removeEventListener('tick', getData) + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [keyLabel, query, visible]) // returns a filtered `metrics` list. Since currently, the backend can return different // metrics based on filters and existing data, this function validates that the metrics @@ -171,9 +217,7 @@ export default function ListReport({ keyLabel, metrics, colMinWidth = COL_MIN_WI if (state.list && state.list.length > 0) { return (

-
- {renderReportHeader()} -
+
{renderReportHeader()}
{renderReportBody()} @@ -228,7 +272,7 @@ export default function ListReport({ keyLabel, metrics, colMinWidth = COL_MIN_WI function renderBarFor(listItem) { const lightBackground = color || 'bg-green-50' - const metricToPlot = metrics.find(metric => metric.meta.plot).key + const metricToPlot = metrics.find((metric) => metric.meta.plot).key return (
@@ -272,7 +316,7 @@ export default function ListReport({ keyLabel, metrics, colMinWidth = COL_MIN_WI style={{ width: colMinWidth, minWidth: colMinWidth }} > - {metric.renderValue(listItem[metric.key])} + {metric.renderValue(listItem)}
) @@ -281,16 +325,26 @@ export default function ListReport({ keyLabel, metrics, colMinWidth = COL_MIN_WI function renderLoading() { return ( -
-
+
+
+
+
) } function renderNoDataYet() { return ( -
-
No data yet
+
+
+ No data yet +
) } @@ -300,16 +354,26 @@ export default function ListReport({ keyLabel, metrics, colMinWidth = COL_MIN_WI const hideDetails = maybeHideDetails && !moreResultsAvailable const showDetails = !!detailsLinkProps && !state.loading && !hideDetails - return showDetails && + return ( + showDetails && ( + + ) + ) } return ( - +
{state.loading && renderLoading()} - {!state.loading && - {renderReport()} - } + {!state.loading && ( + + {renderReport()} + + )}
) diff --git a/assets/js/dashboard/stats/reports/metric-formatter.ts b/assets/js/dashboard/stats/reports/metric-formatter.ts new file mode 100644 index 000000000..e67f5e04e --- /dev/null +++ b/assets/js/dashboard/stats/reports/metric-formatter.ts @@ -0,0 +1,69 @@ +/** @format */ + +import { Metric } from '../../../types/query-api' +import { formatMoneyShort, formatMoneyLong } from '../../util/money' +import { + numberShortFormatter, + durationFormatter, + percentageFormatter, + numberLongFormatter +} from '../../util/number-formatter' + +export type FormattableMetric = + | Metric + | 'total_visitors' + | 'current_visitors' + | 'exit_rate' + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type ValueType = any + +export const MetricFormatterShort: Record< + FormattableMetric, + (value: ValueType) => string +> = { + events: numberShortFormatter, + pageviews: numberShortFormatter, + total_visitors: numberShortFormatter, + current_visitors: numberShortFormatter, + views_per_visit: numberShortFormatter, + visitors: numberShortFormatter, + visits: numberShortFormatter, + + time_on_page: durationFormatter, + visit_duration: durationFormatter, + + bounce_rate: percentageFormatter, + conversion_rate: percentageFormatter, + exit_rate: percentageFormatter, + group_conversion_rate: percentageFormatter, + percentage: percentageFormatter, + + average_revenue: formatMoneyShort, + total_revenue: formatMoneyShort +} + +export const MetricFormatterLong: Record< + FormattableMetric, + (value: ValueType) => string +> = { + events: numberLongFormatter, + pageviews: numberLongFormatter, + total_visitors: numberLongFormatter, + current_visitors: numberShortFormatter, + views_per_visit: numberLongFormatter, + visitors: numberLongFormatter, + visits: numberLongFormatter, + + time_on_page: durationFormatter, + visit_duration: durationFormatter, + + bounce_rate: percentageFormatter, + conversion_rate: percentageFormatter, + exit_rate: percentageFormatter, + group_conversion_rate: percentageFormatter, + percentage: percentageFormatter, + + average_revenue: formatMoneyLong, + total_revenue: formatMoneyLong +} diff --git a/assets/js/dashboard/stats/reports/metric-value.test.tsx b/assets/js/dashboard/stats/reports/metric-value.test.tsx new file mode 100644 index 000000000..612187bb8 --- /dev/null +++ b/assets/js/dashboard/stats/reports/metric-value.test.tsx @@ -0,0 +1,212 @@ +/** @format */ + +import React from 'react' +import { + render as libraryRender, + screen, + fireEvent, + waitFor +} from '@testing-library/react' +import MetricValue from './metric-value' +import SiteContextProvider, { PlausibleSite } from '../../site-context' + +const REVENUE = { long: '$1,659.50', short: '$1.7K' } + +describe('single value', () => { + it('renders small value', async () => { + await renderWithTooltip() + + expect(screen.getByTestId('metric-value')).toHaveTextContent('10') + expect(screen.getByRole('tooltip')).toHaveTextContent('10') + }) + + it('renders large value', async () => { + await renderWithTooltip() + + expect(screen.getByTestId('metric-value')).toHaveTextContent('12.3k') + expect(screen.getByRole('tooltip')).toHaveTextContent('12,345') + }) + + it('renders percentages', async () => { + await renderWithTooltip() + + expect(screen.getByTestId('metric-value')).toHaveTextContent('5.3%') + expect(screen.getByRole('tooltip')).toHaveTextContent('5.3%') + }) + + it('renders durations', async () => { + await renderWithTooltip( + + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('1m 00s') + expect(screen.getByRole('tooltip')).toHaveTextContent('1m 00s') + }) + + it('renders with custom formatter', async () => { + await renderWithTooltip( + `${value}$`} + /> + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('5.3$') + expect(screen.getByRole('tooltip')).toHaveTextContent('5.3$') + }) + + it('renders revenue properly', async () => { + await renderWithTooltip( + + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('$1.7K') + expect(screen.getByRole('tooltip')).toHaveTextContent('$1,659.50') + }) + + it('renders null revenue without tooltip', async () => { + render() + + expect(screen.getByTestId('metric-value')).toHaveTextContent('-') + + await expect(waitForTooltip).rejects.toThrow() + }) +}) + +describe('comparisons', () => { + it('renders increased metric', async () => { + await renderWithTooltip( + + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('10↑') + expect(screen.getByRole('tooltip')).toHaveTextContent( + '10 vs. 5 visitors↑ 100%' + ) + }) + + it('renders decreased metric', async () => { + await renderWithTooltip( + + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('5↓') + expect(screen.getByRole('tooltip')).toHaveTextContent( + '5 vs. 10 visitors↓ 50%' + ) + }) + + it('renders unchanged metric', async () => { + await renderWithTooltip( + + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('10〰') + expect(screen.getByRole('tooltip')).toHaveTextContent( + '10 vs. 10 visitors〰 0%' + ) + }) + + it('renders metric with custom label', async () => { + await renderWithTooltip( + 'Conversions'} + /> + ) + + expect(screen.getByRole('tooltip')).toHaveTextContent( + '10 vs. 10 conversions〰 0%' + ) + }) + + it('does not render very short labels', async () => { + await renderWithTooltip( + '%'} + /> + ) + + expect(screen.getByRole('tooltip')).toHaveTextContent('10% vs. 10%〰 0%') + }) + + it('renders with custom formatter', async () => { + await renderWithTooltip( + `${value}$`} + /> + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('10$↑') + expect(screen.getByRole('tooltip')).toHaveTextContent( + '10$ vs. 5$ test↑ 100%' + ) + }) + + it('renders revenue change', async () => { + await renderWithTooltip( + + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('$1.7K〰') + expect(screen.getByRole('tooltip')).toHaveTextContent( + '$1,659.50 vs. $1,659.50 average_revenue〰 0%' + ) + }) + + it('renders without tooltip when revenue null', async () => { + render( + + ) + + expect(screen.getByTestId('metric-value')).toHaveTextContent('-') + + await expect(waitForTooltip).rejects.toThrow() + }) +}) + +function valueProps( + metric: string, + value: T, + comparison?: { value: T; change: number } +) { + return { + metric: metric, + listItem: { + [metric]: value, + comparison: comparison && { + [metric]: comparison.value, + change: { + [metric]: comparison.change + } + } + }, + renderLabel: (_query: unknown) => metric.toUpperCase() + } as any /* eslint-disable-line @typescript-eslint/no-explicit-any */ +} + +function render(ui: React.ReactNode) { + const site = { + flags: { breakdown_comparisons_ui: true } + } as unknown as PlausibleSite + libraryRender({ui}) +} + +async function renderWithTooltip(ui: React.ReactNode) { + render(ui) + await waitForTooltip() +} + +async function waitForTooltip() { + fireEvent.mouseOver(screen.getByTestId('metric-value')) + await waitFor(() => screen.getByRole('tooltip')) +} diff --git a/assets/js/dashboard/stats/reports/metric-value.tsx b/assets/js/dashboard/stats/reports/metric-value.tsx new file mode 100644 index 000000000..b9187ef0e --- /dev/null +++ b/assets/js/dashboard/stats/reports/metric-value.tsx @@ -0,0 +1,125 @@ +/** @format */ + +import React, { useMemo } from 'react' +import { Metric } from '../../../types/query-api' +import { Tooltip } from '../../util/tooltip' +import { ChangeArrow } from './change-arrow' +import { + MetricFormatterLong, + MetricFormatterShort, + ValueType +} from './metric-formatter' +import { DashboardQuery } from '../../query' +import { useQueryContext } from '../../query-context' +import { PlausibleSite, useSiteContext } from '../../site-context' + +type MetricValues = Record + +type ListItem = MetricValues & { + comparison: MetricValues & { change: Record } +} + +function valueRenderProps( + listItem: ListItem, + metric: Metric, + site: PlausibleSite +) { + const value = listItem[metric] + + let comparison = null + if (site.flags.breakdown_comparisons_ui && listItem.comparison) { + comparison = { + value: listItem.comparison[metric], + change: listItem.comparison.change[metric] + } + } + + return { value, comparison } +} + +export default function MetricValue(props: { + listItem: ListItem + metric: Metric + renderLabel: (query: DashboardQuery) => string + formatter?: (value: ValueType) => string +}) { + const { query } = useQueryContext() + const site = useSiteContext() + + const { metric, listItem } = props + const { value, comparison } = useMemo( + () => valueRenderProps(listItem, metric, site), + [listItem, metric, site] + ) + const metricLabel = useMemo(() => props.renderLabel(query), [query, props]) + const shortFormatter = props.formatter ?? MetricFormatterShort[metric] + + if (value === null && (!comparison || comparison.value === null)) { + return {shortFormatter(value)} + } + + return ( + + } + > + + {shortFormatter(value)} + {comparison ? ( + + ) : null} + + + ) +} + +function ComparisonTooltipContent({ + value, + comparison, + metric, + metricLabel, + formatter +}: { + value: ValueType + comparison: { value: ValueType; change: number } | null + metric: Metric + metricLabel: string + formatter?: (value: ValueType) => string +}) { + const longFormatter = formatter ?? MetricFormatterLong[metric] + + const label = useMemo(() => { + if (metricLabel.length < 3) { + return '' + } + + return ` ${metricLabel.toLowerCase()}` + }, [metricLabel]) + + if (comparison) { + return ( +
+ {longFormatter(value)} vs. {longFormatter(comparison.value)} + {label} + +
+ ) + } else { + return
{longFormatter(value)}
+ } +} diff --git a/assets/js/dashboard/stats/reports/metrics.js b/assets/js/dashboard/stats/reports/metrics.js index 350f737d1..81a161395 100644 --- a/assets/js/dashboard/stats/reports/metrics.js +++ b/assets/js/dashboard/stats/reports/metrics.js @@ -1,19 +1,8 @@ -import { hasGoalFilter } from "../../util/filters" -import numberFormatter, { durationFormatter, percentageFormatter } from "../../util/number-formatter" -import React from "react" +/** @format */ -/*global BUILD_EXTRA*/ -/*global require*/ -function maybeRequire() { - if (BUILD_EXTRA) { - // eslint-disable-next-line @typescript-eslint/no-require-imports - return require('../../extra/money') - } else { - return { default: null } - } -} - -const Money = maybeRequire().default +import React from 'react' +import MetricValue from './metric-value' +import { hasGoalFilter } from '../../util/filters' // Class representation of a metric. @@ -25,11 +14,11 @@ const Money = maybeRequire().default // * `key` - the key under which to read values under in an API -// * `renderValue` - a function that takes a value of this metric, and +// * `formatter` - a function that takes a value of this metric, and // and returns the "rendered" version of it. Can be JSX or a string. // * `renderLabel` - a function rendering a label for this metric given a -// query argument. Can return JSX or string. +// query argument. Returns string. // ### Optional props @@ -39,21 +28,32 @@ const Money = maybeRequire().default export class Metric { constructor(props) { if (!props.key) { - throw Error("Required field `key` is missing") + throw Error('Required field `key` is missing') } if (typeof props.renderLabel !== 'function') { - throw Error("Required field `renderLabel` should be a function") - } - if (typeof props.renderValue !== 'function') { - throw Error("Required field `renderValue` should be a function") + throw Error('Required field `renderLabel` should be a function') } this.key = props.key - this.renderValue = props.renderValue - this.renderLabel = props.renderLabel this.meta = props.meta || {} this.sortable = props.sortable this.width = props.width ?? 'w-24' + + this.formatter = props.formatter + this.renderLabel = props.renderLabel + + this.renderValue = this.renderValue.bind(this) + } + + renderValue(listItem) { + return ( + + ) } } @@ -65,16 +65,8 @@ export class Metric { // * `realtimeLabel` - label when realtime period // * `goalFilterLabel` - label when goal filter is applied export const createVisitors = (props) => { - let renderValue - - if (typeof props.renderValue === 'function') { - renderValue = props.renderValue - } else { - renderValue = renderNumberWithTooltip - } - let renderLabel - + if (typeof props.renderLabel === 'function') { renderLabel = props.renderLabel } else { @@ -83,85 +75,139 @@ export const createVisitors = (props) => { const realtimeLabel = props.realtimeLabel || 'Current visitors' const goalFilterLabel = props.goalFilterLabel || 'Conversions' - if (query.period === 'realtime') { return realtimeLabel } - if (query && hasGoalFilter(query)) { return goalFilterLabel } + if (query.period === 'realtime') { + return realtimeLabel + } + if (query && hasGoalFilter(query)) { + return goalFilterLabel + } return defaultLabel } } - return new Metric({width: 'w-24', sortable: true, ...props, key: "visitors", renderValue, renderLabel}) + return new Metric({ + width: 'w-24', + sortable: true, + ...props, + key: 'visitors', + renderLabel + }) } export const createConversionRate = (props) => { - const renderValue = percentageFormatter - const renderLabel = (_query) => "CR" - return new Metric({width: 'w-16', ...props, key: "conversion_rate", renderLabel, renderValue, sortable: true}) + const renderLabel = (_query) => 'CR' + return new Metric({ + width: 'w-16', + ...props, + key: 'conversion_rate', + renderLabel, + sortable: true + }) } export const createPercentage = (props) => { - const renderValue = (value) => value - const renderLabel = (_query) => "%" - return new Metric({width: 'w-16', ...props, key: "percentage", renderLabel, renderValue, sortable: true}) + const renderLabel = (_query) => '%' + return new Metric({ + width: 'w-16', + ...props, + key: 'percentage', + renderLabel, + sortable: true + }) } export const createEvents = (props) => { - const renderValue = typeof props.renderValue === 'function' ? props.renderValue : renderNumberWithTooltip - return new Metric({width: 'w-24', ...props, key: "events", renderValue: renderValue, sortable: true}) + return new Metric({ width: 'w-24', ...props, key: 'events', sortable: true }) } export const createTotalRevenue = (props) => { - const renderValue = (value) => - const renderLabel = (_query) => "Revenue" - return new Metric({width: 'w-24', ...props, key: "total_revenue", renderValue, renderLabel, sortable: true}) + const renderLabel = (_query) => 'Revenue' + return new Metric({ + width: 'w-24', + ...props, + key: 'total_revenue', + renderLabel, + sortable: true + }) } export const createAverageRevenue = (props) => { - const renderValue = (value) => - const renderLabel = (_query) => "Average" - return new Metric({width: 'w-24', ...props, key: "average_revenue", renderValue, renderLabel, sortable: true}) + const renderLabel = (_query) => 'Average' + return new Metric({ + width: 'w-24', + ...props, + key: 'average_revenue', + renderLabel, + sortable: true + }) } export const createTotalVisitors = (props) => { - const renderValue = renderNumberWithTooltip - const renderLabel = (_query) => "Total Visitors" - return new Metric({width: 'w-28', ...props, key: "total_visitors", renderValue, renderLabel, sortable: false}) + const renderLabel = (_query) => 'Total Visitors' + return new Metric({ + width: 'w-28', + ...props, + key: 'total_visitors', + renderLabel, + sortable: false + }) } export const createVisits = (props) => { - const renderValue = renderNumberWithTooltip - return new Metric({width: 'w-24', sortable: true, ...props, key: "visits", renderValue }) + return new Metric({ width: 'w-24', sortable: true, ...props, key: 'visits' }) } export const createVisitDuration = (props) => { - const renderValue = durationFormatter - const renderLabel = (_query) => "Visit Duration" - return new Metric({width: 'w-36', ...props, key: "visit_duration", renderValue, renderLabel, sortable: true}) + const renderLabel = (_query) => 'Visit Duration' + return new Metric({ + width: 'w-36', + ...props, + key: 'visit_duration', + renderLabel, + sortable: true + }) } export const createBounceRate = (props) => { - const renderValue = (value) => `${value}%` - const renderLabel = (_query) => "Bounce Rate" - return new Metric({width: 'w-32', ...props, key: "bounce_rate", renderValue, renderLabel, sortable: true}) + const renderLabel = (_query) => 'Bounce Rate' + return new Metric({ + width: 'w-32', + ...props, + key: 'bounce_rate', + renderLabel, + sortable: true + }) } export const createPageviews = (props) => { - const renderValue = renderNumberWithTooltip - const renderLabel = (_query) => "Pageviews" - return new Metric({width: 'w-28', ...props, key: "pageviews", renderValue, renderLabel, sortable: true}) + const renderLabel = (_query) => 'Pageviews' + return new Metric({ + width: 'w-28', + ...props, + key: 'pageviews', + renderLabel, + sortable: true + }) } export const createTimeOnPage = (props) => { - const renderValue = durationFormatter - const renderLabel = (_query) => "Time on Page" - return new Metric({width: 'w-32', ...props, key: "time_on_page", renderValue, renderLabel, sortable: false}) + const renderLabel = (_query) => 'Time on Page' + return new Metric({ + width: 'w-32', + ...props, + key: 'time_on_page', + renderLabel, + sortable: false + }) } export const createExitRate = (props) => { - const renderValue = percentageFormatter - const renderLabel = (_query) => "Exit Rate" - return new Metric({width: 'w-28', ...props, key: "exit_rate", renderValue, renderLabel, sortable: false}) + const renderLabel = (_query) => 'Exit Rate' + return new Metric({ + width: 'w-28', + ...props, + key: 'exit_rate', + renderLabel, + sortable: false + }) } - -export function renderNumberWithTooltip(value) { - return {numberFormatter(value)} -} \ No newline at end of file diff --git a/assets/js/dashboard/stats/sources/search-terms.js b/assets/js/dashboard/stats/sources/search-terms.js index eb360f33e..cf38a5edb 100644 --- a/assets/js/dashboard/stats/sources/search-terms.js +++ b/assets/js/dashboard/stats/sources/search-terms.js @@ -2,7 +2,7 @@ import React from 'react'; import FadeIn from '../../fade-in' import Bar from '../bar' import MoreLink from '../more-link' -import numberFormatter from '../../util/number-formatter' +import { numberShortFormatter } from '../../util/number-formatter' import RocketIcon from '../modals/rocket-icon' import * as api from '../../api' import LazyLoader from '../../components/lazy-loader' @@ -69,7 +69,7 @@ export default class SearchTerms extends React.Component { - {numberFormatter(term.visitors)} + {numberShortFormatter(term.visitors)}
) } diff --git a/assets/js/dashboard/util/money.ts b/assets/js/dashboard/util/money.ts new file mode 100644 index 000000000..c329a3d39 --- /dev/null +++ b/assets/js/dashboard/util/money.ts @@ -0,0 +1,17 @@ +type Money = { long: string, short: string } + +export function formatMoneyShort(value: Money | null) { + if (value) { + return value.short + } else { + return "-" + } +} + +export function formatMoneyLong(value: Money | null) { + if (value) { + return value.long + } else { + return "-" + } +} diff --git a/assets/js/dashboard/util/number-formatter.test.ts b/assets/js/dashboard/util/number-formatter.test.ts new file mode 100644 index 000000000..ff2615a0e --- /dev/null +++ b/assets/js/dashboard/util/number-formatter.test.ts @@ -0,0 +1,33 @@ +import { numberLongFormatter, numberShortFormatter } from "./number-formatter" + +describe("numberShortFormatter()", () => { + it('converts to short format', () => { + expect(numberShortFormatter(0)).toEqual('0') + expect(numberShortFormatter(-10)).toEqual('-10') + expect(numberShortFormatter(12)).toEqual('12') + expect(numberShortFormatter(123)).toEqual('123') + expect(numberShortFormatter(1234)).toEqual('1.2k') + expect(numberShortFormatter(12345)).toEqual('12.3k') + expect(numberShortFormatter(123456)).toEqual('123k') + expect(numberShortFormatter(1234567)).toEqual('1.2M') + expect(numberShortFormatter(12345678)).toEqual('12.3M') + expect(numberShortFormatter(123456789)).toEqual('123M') + expect(numberShortFormatter(1234567890)).toEqual('1.2B') + }) +}) + +describe("numberLongFormatter()", () => { + it('converts to short format', () => { + expect(numberLongFormatter(0)).toEqual('0') + expect(numberLongFormatter(-10)).toEqual('-10') + expect(numberLongFormatter(12)).toEqual('12') + expect(numberLongFormatter(123)).toEqual('123') + expect(numberLongFormatter(1234)).toEqual('1,234') + expect(numberLongFormatter(12345)).toEqual('12,345') + expect(numberLongFormatter(123456)).toEqual('123,456') + expect(numberLongFormatter(1234567)).toEqual('1,234,567') + expect(numberLongFormatter(12345678)).toEqual('12,345,678') + expect(numberLongFormatter(123456789)).toEqual('123,456,789') + expect(numberLongFormatter(1234567890)).toEqual('1,234,567,890') + }) +}) diff --git a/assets/js/dashboard/util/number-formatter.js b/assets/js/dashboard/util/number-formatter.ts similarity index 78% rename from assets/js/dashboard/util/number-formatter.js rename to assets/js/dashboard/util/number-formatter.ts index 48ced1e43..9dfc4fd94 100644 --- a/assets/js/dashboard/util/number-formatter.js +++ b/assets/js/dashboard/util/number-formatter.ts @@ -6,7 +6,9 @@ const BILLION = 1000000000 const HUNDRED_BILLION = 100000000000 const TRILLION = 1000000000000 -export default function numberFormatter(num) { +const numberFormat = Intl.NumberFormat("en-US") + +export function numberShortFormatter(num: number): string { if (num >= THOUSAND && num < MILLION) { const thousands = num / THOUSAND if (thousands === Math.floor(thousands) || num >= HUNDRED_THOUSAND) { @@ -29,15 +31,19 @@ export default function numberFormatter(num) { return (Math.floor(billions * 10) / 10) + 'B' } } else { - return num + return num.toString() } } -function pad(num, size) { +export function numberLongFormatter(num: number): string { + return numberFormat.format(num) +} + +function pad(num: number, size: number): string { return ('000' + num).slice(size * -1); } -export function durationFormatter(duration) { +export function durationFormatter(duration: number): string { const hours = Math.floor(duration / 60 / 60) const minutes = Math.floor(duration / 60) % 60 const seconds = Math.floor(duration - (minutes * 60) - (hours * 60 * 60)) @@ -50,7 +56,7 @@ export function durationFormatter(duration) { } } -export function percentageFormatter(number) { +export function percentageFormatter(number: number | null): string { if (typeof (number) === 'number') { return number + '%' } else { diff --git a/assets/js/dashboard/util/tooltip.js b/assets/js/dashboard/util/tooltip.tsx similarity index 71% rename from assets/js/dashboard/util/tooltip.js rename to assets/js/dashboard/util/tooltip.tsx index 8cfc77a28..4354954a1 100644 --- a/assets/js/dashboard/util/tooltip.js +++ b/assets/js/dashboard/util/tooltip.tsx @@ -1,12 +1,19 @@ -import React, { useState } from "react"; +import React, { ReactNode, useState } from "react"; import { usePopper } from 'react-popper'; import classNames from 'classnames' -export function Tooltip({ children, info, className, onClick, boundary }) { +export function Tooltip({ children, info, className, onClick, boundary }: { + info: ReactNode, + children: ReactNode + className?: string, + onClick?: () => void, + boundary?: HTMLElement +}) { const [visible, setVisible] = useState(false); - const [referenceElement, setReferenceElement] = useState(null); - const [popperElement, setPopperElement] = useState(null); - const [arrowElement, setArrowElement] = useState(null); + const [referenceElement, setReferenceElement] = useState(null) + const [popperElement, setPopperElement] = useState(null) + const [arrowElement, setArrowElement] = useState(null) + const { styles, attributes } = usePopper(referenceElement, popperElement, { placement: 'top', modifiers: [ @@ -23,7 +30,7 @@ export function Tooltip({ children, info, className, onClick, boundary }) { boundary: boundary, }, }, - ], + ].filter((x) => !!x), }); return ( diff --git a/extra/lib/plausible/stats/goal/revenue.ex b/extra/lib/plausible/stats/goal/revenue.ex index 3a1948c0e..08da84d8a 100644 --- a/extra/lib/plausible/stats/goal/revenue.ex +++ b/extra/lib/plausible/stats/goal/revenue.ex @@ -47,7 +47,13 @@ defmodule Plausible.Stats.Goal.Revenue do get_goal_dimension_revenue_currency(query, dimension_values) if currency do - Money.new!(value || 0, currency) + money = Money.new!(value || 0, currency) + + %{ + short: Money.to_string!(money, format: :short, fractional_digits: 1), + long: Money.to_string!(money), + value: Decimal.to_float(money.amount) + } else value end diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex index 85e5d78f9..81c2dddfc 100644 --- a/lib/plausible/stats/breakdown.ex +++ b/lib/plausible/stats/breakdown.ex @@ -43,17 +43,31 @@ defmodule Plausible.Stats.Breakdown do end defp build_breakdown_result(query_result, query, metrics) do + dimension_keys = query.dimensions |> Enum.map(&result_key/1) + query_result.results - |> Enum.map(fn %{dimensions: dimensions, metrics: entry_metrics} -> - dimension_map = - query.dimensions |> Enum.map(&result_key/1) |> Enum.zip(dimensions) |> Enum.into(%{}) + |> Enum.map(fn entry -> + comparison_map = + if entry[:comparison] do + comparison = + build_map(metrics, entry.comparison.metrics) + |> Map.put(:change, build_map(metrics, entry.comparison.change)) - metrics_map = Enum.zip(metrics, entry_metrics) |> Enum.into(%{}) + %{comparison: comparison} + else + %{} + end - Map.merge(dimension_map, metrics_map) + build_map(dimension_keys, entry.dimensions) + |> Map.merge(build_map(metrics, entry.metrics)) + |> Map.merge(comparison_map) end) end + defp build_map(keys, values) do + Enum.zip(keys, values) |> Map.new() + end + defp result_key("event:props:" <> custom_property), do: custom_property defp result_key("event:" <> key), do: key |> String.to_existing_atom() defp result_key("visit:" <> key), do: key |> String.to_existing_atom() diff --git a/lib/plausible/stats/compare.ex b/lib/plausible/stats/compare.ex index f00bdd76e..7acb81985 100644 --- a/lib/plausible/stats/compare.ex +++ b/lib/plausible/stats/compare.ex @@ -14,13 +14,7 @@ defmodule Plausible.Stats.Compare do def percent_change(nil, _new_count), do: nil def percent_change(_old_count, nil), do: nil - def percent_change(%Money{} = old_count, %Money{} = new_count) do - percent_change(old_count |> Money.to_decimal(), new_count |> Money.to_decimal()) - end - - def percent_change(%Decimal{} = old_count, %Decimal{} = new_count) do - old_count = old_count |> Decimal.to_float() - new_count = new_count |> Decimal.to_float() + def percent_change(%{value: old_count}, %{value: new_count}) do percent_change(old_count, new_count) end diff --git a/lib/plausible/stats/legacy/legacy_query_builder.ex b/lib/plausible/stats/legacy/legacy_query_builder.ex index 7b856fc9e..2383d89fb 100644 --- a/lib/plausible/stats/legacy/legacy_query_builder.ex +++ b/lib/plausible/stats/legacy/legacy_query_builder.ex @@ -21,6 +21,7 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do |> put_parsed_filters(params) |> put_preloaded_goals(site) |> put_order_by(params) + |> put_include_comparisons(site, params) |> Query.put_experimental_reduced_joins(site, params) |> Query.put_imported_opts(site, params) @@ -196,6 +197,11 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do end end + defp put_include_comparisons(query, site, params) do + comparisons = parse_comparison_params(site, params) + struct!(query, include: Map.put(query.include, :comparisons, comparisons)) + end + @doc """ ### Examples: iex> QueryBuilder.parse_order_by(nil) @@ -276,4 +282,31 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do _ -> today(query) end end + + def parse_comparison_params(_site, %{"period" => period}) when period in ~w(realtime all), + do: nil + + def parse_comparison_params(_site, %{"comparison" => mode} = params) + when mode in ["previous_period", "year_over_year"] do + %{ + mode: mode, + match_day_of_week: params["match_day_of_week"] == "true" + } + end + + def parse_comparison_params(site, %{"comparison" => "custom"} = params) do + {:ok, date_range} = + Filters.QueryParser.parse_date_range_pair(site, [ + params["compare_from"], + params["compare_to"] + ]) + + %{ + mode: "custom", + date_range: date_range, + match_day_of_week: params["match_day_of_week"] == "true" + } + end + + def parse_comparison_params(_site, _options), do: nil end diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index 9f6e056b9..f32694dd2 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -105,9 +105,7 @@ defmodule PlausibleWeb.Api.StatsController do :ok <- validate_interval(params), :ok <- validate_interval_granularity(site, params, dates), params <- realtime_period_to_30m(params), - query = - Query.from(site, params, debug_metadata(conn)) - |> Query.set_include(:comparisons, parse_comparison_options(site, params)), + query = Query.from(site, params, debug_metadata(conn)), {:ok, metric} <- parse_and_validate_graph_metric(params, query) do {timeseries_result, comparison_result, _meta} = Stats.timeseries(site, query, [metric]) @@ -133,7 +131,7 @@ defmodule PlausibleWeb.Api.StatsController do Enum.map(timeseries, fn row -> case row[metric] do nil -> 0 - %Money{} = money -> Decimal.to_float(money.amount) + %{value: value} -> value value -> value end end) @@ -198,12 +196,10 @@ defmodule PlausibleWeb.Api.StatsController do params = realtime_period_to_30m(params) - query = - Query.from(site, params, debug_metadata(conn)) - |> Query.set_include(:comparisons, parse_comparison_options(site, params)) + query = Query.from(site, params, debug_metadata(conn)) {top_stats, sample_percent} = fetch_top_stats(site, query) - comparison_query = comparison_query(site, query, params) + comparison_query = comparison_query(query) json(conn, %{ top_stats: top_stats, @@ -212,8 +208,8 @@ defmodule PlausibleWeb.Api.StatsController do with_imported_switch: with_imported_switch_info(query, comparison_query), includes_imported: includes_imported?(query, comparison_query), imports_exist: site.complete_import_ids != [], - comparing_from: comparison_query && Query.date_range(comparison_query).first, - comparing_to: comparison_query && Query.date_range(comparison_query).last, + comparing_from: query.include.comparisons && Query.date_range(comparison_query).first, + comparing_to: query.include.comparisons && Query.date_range(comparison_query).last, from: Query.date_range(query).first, to: Query.date_range(query).last }) @@ -326,6 +322,7 @@ defmodule PlausibleWeb.Api.StatsController do stats = [ %{ name: "Current visitors", + graph_metric: :current_visitors, value: Stats.current_visitors(site) }, %{ @@ -354,6 +351,7 @@ defmodule PlausibleWeb.Api.StatsController do stats = [ %{ name: "Current visitors", + graph_metric: :current_visitors, value: Stats.current_visitors(site) }, %{ @@ -379,21 +377,15 @@ defmodule PlausibleWeb.Api.StatsController do [ top_stats_entry(results, "Unique visitors", :total_visitors), - top_stats_entry(results, "Unique conversions", :visitors, graphable?: true), - top_stats_entry(results, "Total conversions", :events, graphable?: true), + top_stats_entry(results, "Unique conversions", :visitors), + top_stats_entry(results, "Total conversions", :events), on_ee do - top_stats_entry(results, "Average revenue", :average_revenue, - formatter: &format_money/1, - graphable?: true - ) + top_stats_entry(results, "Average revenue", :average_revenue) end, on_ee do - top_stats_entry(results, "Total revenue", :total_revenue, - formatter: &format_money/1, - graphable?: true - ) + top_stats_entry(results, "Total revenue", :total_revenue) end, - top_stats_entry(results, "Conversion rate", :conversion_rate, graphable?: true) + top_stats_entry(results, "Conversion rate", :conversion_rate) ] |> Enum.reject(&is_nil/1) |> then(&{&1, 100}) @@ -415,12 +407,12 @@ defmodule PlausibleWeb.Api.StatsController do stats = [ - top_stats_entry(current_results, "Unique visitors", :visitors, graphable?: true), - top_stats_entry(current_results, "Total visits", :visits, graphable?: true), - top_stats_entry(current_results, "Total pageviews", :pageviews, graphable?: true), - top_stats_entry(current_results, "Views per visit", :views_per_visit, graphable?: true), - top_stats_entry(current_results, "Bounce rate", :bounce_rate, graphable?: true), - top_stats_entry(current_results, "Visit duration", :visit_duration, graphable?: true), + top_stats_entry(current_results, "Unique visitors", :visitors), + top_stats_entry(current_results, "Total visits", :visits), + top_stats_entry(current_results, "Total pageviews", :pageviews), + top_stats_entry(current_results, "Views per visit", :views_per_visit), + top_stats_entry(current_results, "Bounce rate", :bounce_rate), + top_stats_entry(current_results, "Visit duration", :visit_duration), top_stats_entry(current_results, "Time on page", :time_on_page, formatter: fn nil -> 0 @@ -438,20 +430,11 @@ defmodule PlausibleWeb.Api.StatsController do formatter = Keyword.get(opts, :formatter, & &1) value = get_in(current_results, [key, :value]) - %{name: name, value: formatter.(value)} - |> maybe_put_graph_metric(opts, key) + %{name: name, value: formatter.(value), graph_metric: key} |> maybe_put_comparison(current_results, key, formatter) end end - defp maybe_put_graph_metric(entry, opts, key) do - if Keyword.get(opts, :graphable?) do - entry |> Map.put(:graph_metric, key) - else - entry - end - end - defp maybe_put_comparison(entry, results, key, formatter) do prev_value = get_in(results, [key, :comparison_value]) change = get_in(results, [key, :change]) @@ -1274,11 +1257,6 @@ defmodule PlausibleWeb.Api.StatsController do site |> Stats.breakdown(query, metrics, pagination) |> transform_keys(%{goal: :name}) - |> Enum.map(fn goal -> - goal - |> Enum.map(&format_revenue_metric/1) - |> Map.new() - end) if params["csv"] do to_csv(conversions, [:name, :visitors, :events], [ @@ -1361,10 +1339,6 @@ defmodule PlausibleWeb.Api.StatsController do props = Stats.breakdown(site, query, metrics, pagination) |> transform_keys(%{prop_key => :name}) - |> Enum.map(fn entry -> - Enum.map(entry, &format_revenue_metric/1) - |> Map.new() - end) %{results: props, skip_imported_reason: query.skip_imported_reason} end @@ -1529,41 +1503,12 @@ defmodule PlausibleWeb.Api.StatsController do |> halt() end - def comparison_query(site, query, params) do - options = parse_comparison_options(site, params) - - if options do - Comparisons.get_comparison_query(query, options) + def comparison_query(query) do + if query.include.comparisons do + Comparisons.get_comparison_query(query, query.include.comparisons) end end - def parse_comparison_options(_site, %{"period" => period}) when period in ~w(realtime all), - do: nil - - def parse_comparison_options(_site, %{"comparison" => mode} = params) - when mode in ["previous_period", "year_over_year"] do - %{ - mode: mode, - match_day_of_week: params["match_day_of_week"] == "true" - } - end - - def parse_comparison_options(site, %{"comparison" => "custom"} = params) do - {:ok, date_range} = - Filters.QueryParser.parse_date_range_pair(site, [ - params["compare_from"], - params["compare_to"] - ]) - - %{ - mode: "custom", - date_range: date_range, - match_day_of_week: params["match_day_of_week"] == "true" - } - end - - def parse_comparison_options(_site, _options), do: nil - defp includes_imported?(source_query, comparison_query) do cond do source_query.include_imported -> true @@ -1572,15 +1517,6 @@ defmodule PlausibleWeb.Api.StatsController do end end - on_ee do - defdelegate format_revenue_metric(metric_value), to: PlausibleWeb.Controllers.API.Revenue - defdelegate format_money(money), to: PlausibleWeb.Controllers.API.Revenue - else - defp format_revenue_metric({metric, value}) do - {metric, value} - end - end - defp breakdown_metrics(query, extra_metrics \\ []) do if Filters.filtering_on_dimension?(query, "event:goal") do [:visitors, :conversion_rate, :total_visitors] diff --git a/lib/plausible_web/controllers/stats_controller.ex b/lib/plausible_web/controllers/stats_controller.ex index 94478299e..62af89173 100644 --- a/lib/plausible_web/controllers/stats_controller.ex +++ b/lib/plausible_web/controllers/stats_controller.ex @@ -367,7 +367,10 @@ defmodule PlausibleWeb.StatsController do defp get_flags(user, site), do: %{ channels: - FunWithFlags.enabled?(:channels, for: user) || FunWithFlags.enabled?(:channels, for: site) + FunWithFlags.enabled?(:channels, for: user) || FunWithFlags.enabled?(:channels, for: site), + breakdown_comparisons_ui: + FunWithFlags.enabled?(:breakdown_comparisons_ui, for: user) || + FunWithFlags.enabled?(:breakdown_comparisons_ui, for: site) } defp is_dbip() do diff --git a/test/plausible_web/controllers/api/stats_controller/conversions_test.exs b/test/plausible_web/controllers/api/stats_controller/conversions_test.exs index fbf0d7dc4..a58687eaf 100644 --- a/test/plausible_web/controllers/api/stats_controller/conversions_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/conversions_test.exs @@ -304,8 +304,16 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do "visitors" => 5, "events" => 5, "conversion_rate" => 100.0, - "average_revenue" => %{"short" => "€166.7M", "long" => "€166,733,566.75"}, - "total_revenue" => %{"short" => "€500.2M", "long" => "€500,200,700.25"} + "average_revenue" => %{ + "short" => "€166.7M", + "long" => "€166,733,566.75", + "value" => 166_733_566.748 + }, + "total_revenue" => %{ + "short" => "€500.2M", + "long" => "€500,200,700.25", + "value" => 500_200_700.246 + } } ] end @@ -380,11 +388,11 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do assert [ %{ - "average_revenue" => %{"long" => "€10.00", "short" => "€10.0"}, + "average_revenue" => %{"long" => "€10.00", "short" => "€10.0", "value" => 10.0}, "conversion_rate" => 16.7, "name" => "Payment", "events" => 1, - "total_revenue" => %{"long" => "€10.00", "short" => "€10.0"}, + "total_revenue" => %{"long" => "€10.00", "short" => "€10.0", "value" => 10.0}, "visitors" => 1 }, %{ diff --git a/test/plausible_web/controllers/api/stats_controller/custom_prop_breakdown_test.exs b/test/plausible_web/controllers/api/stats_controller/custom_prop_breakdown_test.exs index 3e03461f5..33921a905 100644 --- a/test/plausible_web/controllers/api/stats_controller/custom_prop_breakdown_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/custom_prop_breakdown_test.exs @@ -796,16 +796,16 @@ defmodule PlausibleWeb.Api.StatsController.CustomPropBreakdownTest do "name" => "true", "events" => 2, "conversion_rate" => 66.7, - "total_revenue" => %{"long" => "€112.00", "short" => "€112.0"}, - "average_revenue" => %{"long" => "€56.00", "short" => "€56.0"} + "total_revenue" => %{"long" => "€112.00", "short" => "€112.0", "value" => 112.00}, + "average_revenue" => %{"long" => "€56.00", "short" => "€56.0", "value" => 56.00} }, %{ "visitors" => 1, "name" => "false", "events" => 1, "conversion_rate" => 33.3, - "total_revenue" => %{"long" => "€8.00", "short" => "€8.0"}, - "average_revenue" => %{"long" => "€8.00", "short" => "€8.0"} + "total_revenue" => %{"long" => "€8.00", "short" => "€8.0", "value" => 8.00}, + "average_revenue" => %{"long" => "€8.00", "short" => "€8.0", "value" => 8.00} } ] end @@ -857,16 +857,16 @@ defmodule PlausibleWeb.Api.StatsController.CustomPropBreakdownTest do "name" => "true", "events" => 2, "conversion_rate" => 66.7, - "total_revenue" => %{"long" => "€80.00", "short" => "€80.0"}, - "average_revenue" => %{"long" => "€40.00", "short" => "€40.0"} + "total_revenue" => %{"long" => "€80.00", "short" => "€80.0", "value" => 80.0}, + "average_revenue" => %{"long" => "€40.00", "short" => "€40.0", "value" => 40.0} }, %{ "visitors" => 1, "name" => "false", "events" => 1, "conversion_rate" => 33.3, - "total_revenue" => %{"long" => "€10.00", "short" => "€10.0"}, - "average_revenue" => %{"long" => "€10.00", "short" => "€10.0"} + "total_revenue" => %{"long" => "€10.00", "short" => "€10.0", "value" => 10.0}, + "average_revenue" => %{"long" => "€10.00", "short" => "€10.0", "value" => 10.0} } ] end diff --git a/test/plausible_web/controllers/api/stats_controller/top_stats_test.exs b/test/plausible_web/controllers/api/stats_controller/top_stats_test.exs index 3fc92087e..e72087906 100644 --- a/test/plausible_web/controllers/api/stats_controller/top_stats_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/top_stats_test.exs @@ -21,7 +21,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do assert %{"graph_metric" => "visit_duration"} = visit_duration end - test "returns graph_metric key for graphable top stats in realtime mode", %{ + test "returns graph_metric key for top stats in realtime mode", %{ conn: conn, site: site } do @@ -31,7 +31,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do |> json_response(200) |> Map.get("top_stats") - refute Map.has_key?(current_visitors, "graph_metric") + assert %{"graph_metric" => "current_visitors"} = current_visitors assert %{"graph_metric" => "visitors"} = unique_visitors assert %{"graph_metric" => "pageviews"} = pageviews end @@ -189,7 +189,9 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Time on page", "value" => 900} in res["top_stats"] + assert %{"name" => "Time on page", "value" => 900, "graph_metric" => "time_on_page"} in res[ + "top_stats" + ] end test "calculates time on page when filtered for multiple pages", %{ @@ -228,7 +230,9 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Time on page", "value" => 480} in res["top_stats"] + assert %{"name" => "Time on page", "value" => 480, "graph_metric" => "time_on_page"} in res[ + "top_stats" + ] end test "calculates time on page when filtered for multiple negated pages", %{ @@ -267,7 +271,9 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Time on page", "value" => 60} in res["top_stats"] + assert %{"name" => "Time on page", "value" => 60, "graph_metric" => "time_on_page"} in res[ + "top_stats" + ] end test "calculates time on page when filtered for multiple wildcard pages", %{ @@ -307,7 +313,9 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Time on page", "value" => 480} in res["top_stats"] + assert %{"name" => "Time on page", "value" => 480, "graph_metric" => "time_on_page"} in res[ + "top_stats" + ] end test "calculates time on page when filtered for multiple negated wildcard pages", %{ @@ -349,7 +357,9 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Time on page", "value" => 600} in res["top_stats"] + assert %{"name" => "Time on page", "value" => 600, "graph_metric" => "time_on_page"} in res[ + "top_stats" + ] end test "doesn't calculate time on page with only single page visits", %{conn: conn, site: site} do @@ -361,7 +371,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do filters = Jason.encode!(%{page: "/"}) path = "/api/stats/#{site.domain}/top-stats?period=day&date=2021-01-01&filters=#{filters}" - assert %{"name" => "Time on page", "value" => 0} == + assert %{"name" => "Time on page", "value" => 0, "graph_metric" => "time_on_page"} == conn |> get(path) |> json_response(200) @@ -397,7 +407,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do filters = Jason.encode!(%{page: "/"}) path = "/api/stats/#{site.domain}/top-stats?&filters=#{filters}" - assert %{"name" => "Time on page", "value" => 0} == + assert %{"name" => "Time on page", "value" => 0, "graph_metric" => "time_on_page"} == conn |> get(path) |> json_response(200) @@ -416,7 +426,11 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do filters = Jason.encode!(%{page: "/"}) path = "/api/stats/#{site.domain}/top-stats?period=day&date=2021-01-01&filters=#{filters}" - assert %{"name" => "Time on page", "value" => _three_minutes = 180} == + assert %{ + "name" => "Time on page", + "value" => _three_minutes = 180, + "graph_metric" => "time_on_page" + } == conn |> get(path) |> json_response(200) @@ -453,7 +467,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do filters = Jason.encode!(%{page: "/a"}) path = "/api/stats/#{site.domain}/top-stats?period=day&date=2021-01-01&filters=#{filters}" - assert %{"name" => "Time on page", "value" => 100} == + assert %{"name" => "Time on page", "value" => 100, "graph_metric" => "time_on_page"} == conn |> get(path) |> json_response(200) @@ -775,7 +789,10 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do conn = get(conn, "/api/stats/#{site.domain}/top-stats?period=realtime") res = json_response(conn, 200) - assert %{"name" => "Current visitors", "value" => 2} in res["top_stats"] + + assert %{"name" => "Current visitors", "value" => 2, "graph_metric" => "current_visitors"} in res[ + "top_stats" + ] end test "shows unique visitors (last 30 minutes)", %{conn: conn, site: site} do @@ -826,7 +843,10 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do conn = get(conn, "/api/stats/#{site.domain}/top-stats?period=realtime&filters=#{filters}") res = json_response(conn, 200) - assert %{"name" => "Current visitors", "value" => 3} in res["top_stats"] + + assert %{"name" => "Current visitors", "value" => 3, "graph_metric" => "current_visitors"} in res[ + "top_stats" + ] end test "shows unique/total conversions (last 30 min) with goal filter", %{ @@ -865,7 +885,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do describe "GET /api/stats/top-stats - filters" do setup [:create_user, :log_in, :create_new_site] - test "returns graph_metric key for graphable top stats with a page filter", %{ + test "returns graph_metric key for top stats with a page filter", %{ conn: conn, site: site } do @@ -881,11 +901,10 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do assert %{"graph_metric" => "visits"} = visits assert %{"graph_metric" => "pageviews"} = pageviews assert %{"graph_metric" => "bounce_rate"} = bounce_rate - - refute Map.has_key?(time_on_page, "graph_metric") + assert %{"graph_metric" => "time_on_page"} = time_on_page end - test "returns graph_metric key for graphable top stats with a goal filter", %{ + test "returns graph_metric key for top stats with a goal filter", %{ conn: conn, site: site } do @@ -897,13 +916,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do |> json_response(200) |> Map.get("top_stats") - refute Map.has_key?(unique_visitors, "graph_metric") + assert %{"graph_metric" => "total_visitors"} = unique_visitors assert %{"graph_metric" => "visitors"} = unique_conversions assert %{"graph_metric" => "events"} = total_conversions assert %{"graph_metric" => "conversion_rate"} = cr end - test "returns graph_metric key for graphable top stats with a goal filter in realtime mode", + test "returns graph_metric key for top stats with a goal filter in realtime mode", %{conn: conn, site: site} do filters = Jason.encode!(%{goal: "Signup"}) @@ -913,7 +932,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do |> json_response(200) |> Map.get("top_stats") - refute Map.has_key?(current_visitors, "graph_metric") + assert %{"graph_metric" => "current_visitors"} = current_visitors assert %{"graph_metric" => "visitors"} = unique_conversions assert %{"graph_metric" => "events"} = total_conversions end @@ -1258,7 +1277,9 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Unique visitors", "value" => 3} in res["top_stats"] + assert %{"name" => "Unique visitors", "value" => 3, "graph_metric" => "total_visitors"} in res[ + "top_stats" + ] end test "returns converted visitors", %{conn: conn, site: site} do @@ -1381,13 +1402,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do assert %{ "name" => "Average revenue", - "value" => %{"long" => "$1,659.50", "short" => "$1.7K"}, + "value" => %{"long" => "$1,659.50", "short" => "$1.7K", "value" => 1659.5}, "graph_metric" => "average_revenue" } in top_stats assert %{ "name" => "Total revenue", - "value" => %{"long" => "$3,319.00", "short" => "$3.3K"}, + "value" => %{"long" => "$3,319.00", "short" => "$3.3K", "value" => 3319.0}, "graph_metric" => "total_revenue" } in top_stats end @@ -1440,13 +1461,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do assert %{ "name" => "Average revenue", - "value" => %{"long" => "$1,659.50", "short" => "$1.7K"}, + "value" => %{"long" => "$1,659.50", "short" => "$1.7K", "value" => 1659.5}, "graph_metric" => "average_revenue" } in top_stats assert %{ "name" => "Total revenue", - "value" => %{"long" => "$6,638.00", "short" => "$6.6K"}, + "value" => %{"long" => "$6,638.00", "short" => "$6.6K", "value" => 6638.0}, "graph_metric" => "total_revenue" } in top_stats end @@ -1516,13 +1537,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do assert %{ "name" => "Average revenue", - "value" => %{"long" => "$1,000.00", "short" => "$1.0K"}, + "value" => %{"long" => "$1,000.00", "short" => "$1.0K", "value" => 1000.0}, "graph_metric" => "average_revenue" } in top_stats assert %{ "name" => "Total revenue", - "value" => %{"long" => "$2,000.00", "short" => "$2.0K"}, + "value" => %{"long" => "$2,000.00", "short" => "$2.0K", "value" => 2000.0}, "graph_metric" => "total_revenue" } in top_stats end @@ -1538,13 +1559,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do assert %{ "name" => "Average revenue", - "value" => %{"long" => "$0.00", "short" => "$0.0"}, + "value" => %{"long" => "$0.00", "short" => "$0.0", "value" => 0.0}, "graph_metric" => "average_revenue" } in top_stats assert %{ "name" => "Total revenue", - "value" => %{"long" => "$0.00", "short" => "$0.0"}, + "value" => %{"long" => "$0.00", "short" => "$0.0", "value" => 0.0}, "graph_metric" => "total_revenue" } in top_stats end