From ff515c641d79566e453bc0bf37da494be330486c Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Thu, 29 Oct 2020 15:33:37 +0200 Subject: [PATCH] Improve event metadata (#385) * Change from dropdown to tabs for metadata breakdown * Add meta filter to base_query * Refactor: use base_query_w_sessions in top_referrers_for_goal * Do not allow individual metadata filter * Fix conversions report when combining 3 filters: goal, metadata, country * Remember selected metadata key * Compress conversions component --- assets/js/dashboard/filters.js | 4 +- .../js/dashboard/stats/conversions/index.js | 6 +- .../stats/conversions/meta-breakdown.js | 95 ++++++------------- assets/js/dashboard/stats/devices.js | 3 +- lib/plausible/stats/clickhouse.ex | 35 +++++-- 5 files changed, 61 insertions(+), 82 deletions(-) diff --git a/assets/js/dashboard/filters.js b/assets/js/dashboard/filters.js index 8c8163c60..a7399a05a 100644 --- a/assets/js/dashboard/filters.js +++ b/assets/js/dashboard/filters.js @@ -48,7 +48,9 @@ function filterText(key, value, query) { function renderFilter(history, [key, value], query) { function removeFilter() { - history.push({search: removeQueryParam(location.search, key)}) + let newQuery = removeQueryParam(location.search, key) + if (key === 'goal') { newQuery = removeQueryParam(newQuery, 'meta') } + history.push({search: newQuery}) } return ( diff --git a/assets/js/dashboard/stats/conversions/index.js b/assets/js/dashboard/stats/conversions/index.js index f24b1c53a..9d50c5aa4 100644 --- a/assets/js/dashboard/stats/conversions/index.js +++ b/assets/js/dashboard/stats/conversions/index.js @@ -50,13 +50,13 @@ export default class Conversions extends React.Component { return (
-
+
{this.renderGoalText(goal.name)}
{numberFormatter(goal.count)} - {numberFormatter(goal.total_count)} + {numberFormatter(goal.total_count)}
{ renderMeta && } @@ -79,7 +79,7 @@ export default class Conversions extends React.Component { Goal
Uniques - Total conversions + Total
diff --git a/assets/js/dashboard/stats/conversions/meta-breakdown.js b/assets/js/dashboard/stats/conversions/meta-breakdown.js index 9d25ee960..00f4b9c42 100644 --- a/assets/js/dashboard/stats/conversions/meta-breakdown.js +++ b/assets/js/dashboard/stats/conversions/meta-breakdown.js @@ -1,7 +1,6 @@ import React from 'react'; import { Link } from 'react-router-dom' -import Transition from "../../../transition.js"; import Bar from '../bar' import numberFormatter from '../../number-formatter' import * as api from '../../api' @@ -9,31 +8,24 @@ import * as api from '../../api' export default class MetaBreakdown extends React.Component { constructor(props) { super(props) - this.handleClick = this.handleClick.bind(this) - const metaFilter = props.query.filters['meta'] - console.log(metaFilter) - const metaKey = metaFilter ? Object.keys(metaFilter)[0] : props.goal.meta_keys[0] + let metaKey = props.goal.meta_keys[0] + this.storageKey = 'goalMetaTab__' + props.site.domain + props.goal.name + const storedKey = window.localStorage[this.storageKey] + if (props.goal.meta_keys.includes(storedKey)) { + metaKey = storedKey + } + if (props.query.filters['meta']) { + metaKey = Object.keys(props.query.filters['meta'])[0] + } + this.state = { loading: true, - dropdownOpen: false, metaKey: metaKey } } componentDidMount() { this.fetchMetaBreakdown() - document.addEventListener('mousedown', this.handleClick, false); - } - - componentWillUnmount() { - document.removeEventListener('mousedown', this.handleClick, false); - } - - handleClick(e) { - if (this.dropDownNode && this.dropDownNode.contains(e.target)) return; - if (!this.state.dropdownOpen) return; - - this.setState({dropdownOpen: false}) } fetchMetaBreakdown() { @@ -49,7 +41,7 @@ export default class MetaBreakdown extends React.Component { return (
-
+
{ value.name } @@ -57,36 +49,15 @@ export default class MetaBreakdown extends React.Component {
{numberFormatter(value.count)} - {numberFormatter(value.total_count)} + {numberFormatter(value.total_count)}
) } changeMetaKey(newKey) { - this.setState({metaKey: newKey, loading: true, dropdownOpen: false}, this.fetchMetaBreakdown) - } - - renderMetaKeyOption(key) { - const extraClass = key === this.state.metaKey ? 'font-medium text-gray-900' : 'hover:bg-gray-100 hover:text-gray-900 focus:outline-none focus:bg-gray-100 focus:text-gray-900' - - return ( - - {key} - - ) - } - - renderDropdown() { - return ( -
- { this.props.goal.meta_keys.map(this.renderMetaKeyOption.bind(this)) } -
- ) - } - - toggleDropdown() { - this.setState({dropdownOpen: !this.state.dropdownOpen}) + window.localStorage[this.storageKey] = newKey + this.setState({metaKey: newKey, loading: true}, this.fetchMetaBreakdown) } renderBody() { @@ -97,32 +68,24 @@ export default class MetaBreakdown extends React.Component { } } + renderPill(key) { + const isActive = this.state.metaKey === key + + if (isActive) { + return
  • {key}
    • + } else { + return
  • {key}
    • + } + } + render() { return (
    -
    - Breakdown by - - -
    this.dropDownNode = node} > -
    - { this.renderDropdown() } -
    -
    -     +
    + Breakdown by: +
      + { this.props.goal.meta_keys.map(this.renderPill.bind(this)) } +
    { this.renderBody() }
    diff --git a/assets/js/dashboard/stats/devices.js b/assets/js/dashboard/stats/devices.js index 3ec85b44b..cc797c51b 100644 --- a/assets/js/dashboard/stats/devices.js +++ b/assets/js/dashboard/stats/devices.js @@ -291,10 +291,9 @@ export default class Devices extends React.Component { renderPill(name, mode) { const isActive = this.state.mode === mode - const extraClass = name === 'OS' ? '' : ' border-r border-gray-300' if (isActive) { - return
  • {name}
    • + return
  • {name}
    • } else { return
  • {name}
    • } diff --git a/lib/plausible/stats/clickhouse.ex b/lib/plausible/stats/clickhouse.ex index 215e9fe89..f707de5a2 100644 --- a/lib/plausible/stats/clickhouse.ex +++ b/lib/plausible/stats/clickhouse.ex @@ -171,14 +171,8 @@ defmodule Plausible.Stats.Clickhouse do def top_referrers_for_goal(site, query, limit, page) do offset = (page - 1) * limit - converted_sessions = - from(e in base_query(site, query), - select: %{session_id: e.session_id}) - ClickhouseRepo.all( - from s in Plausible.ClickhouseSession, - join: cs in subquery(converted_sessions), - on: s.session_id == cs.session_id, + from s in base_query_w_sessions(site, query), where: s.referrer_source != "", group_by: s.referrer_source, order_by: [desc: fragment("count")], @@ -611,7 +605,7 @@ defmodule Plausible.Stats.Clickhouse do %{goal => [key]} else ClickhouseRepo.all( - from [e, meta] in base_query_w_sessions_bare(site, query), + from [e, meta: meta] in base_query_w_sessions_bare(site, query), select: {e.name, meta.key}, distinct: true ) |> Enum.reduce(%{}, fn {goal_name, meta_key}, acc -> @@ -647,7 +641,7 @@ defmodule Plausible.Stats.Clickhouse do ) else ClickhouseRepo.all( - from [e, meta] in base_query_w_sessions(site, query), + from [e, meta: meta] in base_query_w_sessions(site, query), group_by: meta.value, order_by: [desc: fragment("count")], select: %{ @@ -857,7 +851,7 @@ defmodule Plausible.Stats.Clickhouse do q end - if query.filters["page"] do + q = if query.filters["page"] do page = query.filters["page"] from(e in q, where: e.pathname == ^page) else @@ -876,6 +870,7 @@ defmodule Plausible.Stats.Clickhouse do from( e in q, inner_lateral_join: meta in fragment("meta as m"), + as: :meta, where: meta.key == ^key and meta.value == ^val, ) end @@ -1075,6 +1070,26 @@ defmodule Plausible.Stats.Clickhouse do q end + q = if query.filters["meta"] do + [{key, val}] = query.filters["meta"] |> Enum.into([]) + + if val == "(none)" do + from( + e in q, + where: fragment("not has(meta.key, ?)", ^key) + ) + else + from( + e in q, + inner_lateral_join: meta in fragment("meta as m"), + where: meta.key == ^key and meta.value == ^val, + ) + end + else + q + end + + q = if path do from(e in q, where: e.pathname == ^path)