Fix with imported switch + warning bubble renders (#4187)

* prioritize other skip_imported_reasons over not requested

* return with_imported_switch info from top stats

* fix the with-imported-switch component

* Refactor ImportedQueryUnsupportedWarning

* do not render warning bubble when imported data not requested

* stop displaying warning bubble before the API response is received

* fix tab switch pills jumping due to bubble height

* drop loading condition of ImportedQueryUnsupportedWarning for Funnels

* add explicit check for realtime when rendering the bubble

* remove unused JS imports

* rename snake_case to camelCase
This commit is contained in:
RobertJoonas 2024-06-06 09:50:54 +01:00 committed by GitHub
parent fd81ef2105
commit 2a529b4eaa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 257 additions and 74 deletions

View File

@ -42,13 +42,14 @@ export default function Behaviours(props) {
const funnelKey = `behavioursTabFunnel__${site.domain}`
const [enabledModes, setEnabledModes] = useState(getEnabledModes())
const [mode, setMode] = useState(defaultMode())
const [loading, setLoading] = useState(true)
const [funnelNames, _setFunnelNames] = useState(site.funnels.map(({ name }) => name))
const [selectedFunnel, setSelectedFunnel] = useState(defaultSelectedFunnel())
const [showingPropsForGoalFilter, setShowingPropsForGoalFilter] = useState(false)
const [importedQueryUnsupported, setImportedQueryUnsupported] = useState(false)
const [skipImportedReason, setSkipImportedReason] = useState(null)
const onGoalFilterClick = useCallback((e) => {
const goalName = e.target.innerHTML
@ -73,6 +74,8 @@ export default function Behaviours(props) {
setMode(defaultMode())
}, [enabledModes])
useEffect(() => setLoading(true), [query, mode])
function disableMode(mode) {
setEnabledModes(enabledModes.filter((m) => { return m !== mode }))
}
@ -173,8 +176,8 @@ export default function Behaviours(props) {
}
function afterFetchData(apiResponse) {
const unsupportedQuery = apiResponse.skip_imported_reason === 'unsupported_query'
setImportedQueryUnsupported(unsupportedQuery && !isRealtime())
setLoading(false)
setSkipImportedReason(apiResponse.skip_imported_reason)
}
function renderConversions() {
@ -332,6 +335,16 @@ export default function Behaviours(props) {
}
}
function renderImportedQueryUnsupportedWarning() {
if (mode === CONVERSIONS) {
return <ImportedQueryUnsupportedWarning loading={loading} query={query} skipImportedReason={skipImportedReason}/>
} else if (mode === PROPS) {
return <ImportedQueryUnsupportedWarning loading={loading} query={query} skipImportedReason={skipImportedReason} message="Imported data is unavailable in this view"/>
} else {
return <ImportedQueryUnsupportedWarning altCondition={props.importedDataInView} message="Imported data is unavailable in this view"/>
}
}
if (mode) {
return (
<div className="items-start justify-between block w-full mt-6 md:flex">
@ -341,9 +354,7 @@ export default function Behaviours(props) {
<h3 className="font-bold dark:text-gray-100">
{sectionTitle() + (isRealtime() ? ' (last 30min)' : '')}
</h3>
<ImportedQueryUnsupportedWarning condition={mode === CONVERSIONS && importedQueryUnsupported}/>
<ImportedQueryUnsupportedWarning condition={mode === PROPS && importedQueryUnsupported} message="Imported data is unavailable in this view"/>
<ImportedQueryUnsupportedWarning condition={mode === FUNNELS && props.importedDataInView} message="Imported data is unavailable in this view"/>
{ renderImportedQueryUnsupportedWarning()}
</div>
{tabs()}
</div>

View File

@ -1,4 +1,4 @@
import React, {useState} from 'react';
import React, {useEffect, useState} from 'react';
import * as storage from '../../util/storage'
import { getFiltersByKeyPrefix, isFilteringOnFixedValue } from '../../util/filters'
import ListReport from '../reports/list'
@ -167,7 +167,8 @@ export default function Devices(props) {
const tabKey = `deviceTab__${site.domain}`
const storedTab = storage.getItem(tabKey)
const [mode, setMode] = useState(storedTab || 'browser')
const [importedQueryUnsupported, setImportedQueryUnsupported] = useState(false)
const [loading, setLoading] = useState(true)
const [skipImportedReason, setSkipImportedReason] = useState(null)
function switchTab(mode) {
storage.setItem(tabKey, mode)
@ -175,11 +176,12 @@ export default function Devices(props) {
}
function afterFetchData(apiResponse) {
const unsupportedQuery = apiResponse.skip_imported_reason === 'unsupported_query'
const isRealtime = query.period === 'realtime'
setImportedQueryUnsupported(unsupportedQuery && !isRealtime)
setLoading(false)
setSkipImportedReason(apiResponse.skip_imported_reason)
}
useEffect(() => setLoading(true), [query, mode])
function renderContent() {
switch (mode) {
case 'browser':
@ -226,7 +228,7 @@ export default function Devices(props) {
<div className="flex justify-between w-full">
<div className="flex gap-x-1">
<h3 className="font-bold dark:text-gray-100">Devices</h3>
<ImportedQueryUnsupportedWarning condition={importedQueryUnsupported}/>
<ImportedQueryUnsupportedWarning loading={loading} query={query} skipImportedReason={skipImportedReason}/>
</div>
<div className="flex text-xs font-medium text-gray-500 dark:text-gray-400 space-x-2">
{renderPill('Browser', 'browser')}

View File

@ -143,7 +143,7 @@ export default function VisitorGraph(props) {
<div className="absolute right-4 -top-8 py-1 flex items-center">
{!isRealtime && <StatsExport site={site} query={query} />}
<SamplingNotice samplePercent={topStatData}/>
<WithImportedSwitch site={site} topStatData={topStatData} />
<WithImportedSwitch query={query} info={topStatData && topStatData.with_imported_switch} />
<IntervalPicker site={site} query={query} onIntervalUpdate={onIntervalUpdate} />
</div>
<LineGraphWithRouter graphData={graphData} darkTheme={isDarkTheme} query={query} />

View File

@ -1,38 +1,28 @@
import React from "react"
import { parseNaiveDate, isBefore } from '../../util/date'
import { Link } from 'react-router-dom'
import * as url from '../../util/url'
import { BarsArrowUpIcon } from '@heroicons/react/20/solid'
import classNames from "classnames"
export default function WithImportedSwitch({site, topStatData}) {
if (!topStatData?.imports_exist) {
return null
}
function isBeforeNativeStats(date) {
if (!date) return false
const nativeStatsBegin = parseNaiveDate(site.nativeStatsBegin)
const parsedDate = parseNaiveDate(date)
return isBefore(parsedDate, nativeStatsBegin, "day")
}
const isQueryingImportedPeriod = isBeforeNativeStats(topStatData.from)
const isComparingImportedPeriod = isBeforeNativeStats(topStatData.comparing_from)
if (isQueryingImportedPeriod || isComparingImportedPeriod) {
const withImported = topStatData.includes_imported;
const toggleColor = withImported ? " dark:text-gray-300 text-gray-700" : " dark:text-gray-500 text-gray-400"
const target = url.setQuery('with_imported', (!withImported).toString())
const tip = withImported ? "" : "do not ";
export default function WithImportedSwitch({query, info}) {
if (info && info.visible) {
const {togglable, tooltip_msg} = info
const enabled = togglable && query.with_imported
const target = url.setQuery('with_imported', (!enabled).toString())
const linkClass = classNames({
"dark:text-gray-300 text-gray-700": enabled,
"dark:text-gray-500 text-gray-400": !enabled,
"cursor-pointer": togglable,
"pointer-events-none": !togglable,
})
return (
<Link to={target} className="w-4 h-4 mx-2">
<div tooltip={`Stats ${tip}include imported data.`} className="cursor-pointer w-4 h-4">
<BarsArrowUpIcon className={"absolute " + toggleColor} />
</div>
</Link>
<div tooltip={tooltip_msg} className="w-4 h-4 mx-2">
<Link to={target} className={linkClass}>
<BarsArrowUpIcon className="mt-0.5"/>
</Link>
</div>
)
} else {
return null

View File

@ -1,14 +1,18 @@
import React from "react";
import { ExclamationCircleIcon } from '@heroicons/react/24/outline'
import FadeIn from "../fade-in";
export default function ImportedQueryUnsupportedWarning({condition, message}) {
export default function ImportedQueryUnsupportedWarning({query, loading, skipImportedReason, altCondition, message}) {
const tooltipMessage = message || "Imported data is excluded due to applied filters"
if (condition) {
const show = query && query.with_imported && skipImportedReason === "unsupported_query" && query.period !== 'realtime'
if (show || altCondition) {
return (
<span tooltip={tooltipMessage}>
<ExclamationCircleIcon className="w-6 h-6 dark:text-gray-100" />
</span>
<FadeIn show={!loading} className="h-6">
<span tooltip={tooltipMessage}>
<ExclamationCircleIcon className="w-6 h-6 dark:text-gray-100" />
</span>
</FadeIn>
)
} else {
return null

View File

@ -125,11 +125,12 @@ export default class Locations extends React.Component {
const storedTab = storage.getItem(this.tabKey)
this.state = {
mode: storedTab || 'map',
importedQueryUnsupported: false
loading: true,
skipImportedReason: null
}
}
componentDidUpdate(prevProps) {
componentDidUpdate(prevProps, prevState) {
const isRemovingFilter = (filterName) => {
return getFiltersByKeyPrefix(prevProps.query, filterName).length > 0 &&
getFiltersByKeyPrefix(this.props.query, filterName).length == 0
@ -142,6 +143,10 @@ export default class Locations extends React.Component {
if (this.state.mode === 'regions' && isRemovingFilter('country')) {
this.setMode(this.countriesRestoreMode || 'countries')()
}
if (this.props.query !== prevProps.query || this.state.mode !== prevState.mode) {
this.setState({loading: true})
}
}
setMode(mode) {
@ -163,9 +168,7 @@ export default class Locations extends React.Component {
}
afterFetchData(apiResponse) {
const unsupportedQuery = apiResponse.skip_imported_reason === 'unsupported_query'
const isRealtime = this.props.query.period === 'realtime'
this.setState({importedQueryUnsupported: unsupportedQuery && !isRealtime})
this.setState({loading: false, skipImportedReason: apiResponse.skip_imported_reason})
}
renderContent() {
@ -213,7 +216,7 @@ export default class Locations extends React.Component {
<h3 className="font-bold dark:text-gray-100">
{labelFor[this.state.mode] || 'Locations'}
</h3>
<ImportedQueryUnsupportedWarning condition={this.state.importedQueryUnsupported} />
<ImportedQueryUnsupportedWarning loading={this.state.loading} query={this.props.query} skipImportedReason={this.state.skipImportedReason} />
</div>
<div className="flex text-xs font-medium text-gray-500 dark:text-gray-400 space-x-2">
{ this.renderPill('Map', 'map') }

View File

@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import * as storage from '../../util/storage'
import * as url from '../../util/url'
@ -111,7 +111,8 @@ export default function Pages(props) {
const tabKey = `pageTab__${site.domain}`
const storedTab = storage.getItem(tabKey)
const [mode, setMode] = useState(storedTab || 'pages')
const [importedQueryUnsupported, setImportedQueryUnsupported] = useState(false)
const [loading, setLoading] = useState(true)
const [skipImportedReason, setSkipImportedReason] = useState(null)
function switchTab(mode) {
storage.setItem(tabKey, mode)
@ -119,11 +120,12 @@ export default function Pages(props) {
}
function afterFetchData(apiResponse) {
const unsupportedQuery = apiResponse.skip_imported_reason === 'unsupported_query'
const isRealtime = query.period === 'realtime'
setImportedQueryUnsupported(unsupportedQuery && !isRealtime)
setLoading(false)
setSkipImportedReason(apiResponse.skip_imported_reason)
}
useEffect(() => setLoading(true), [query, mode])
function renderContent() {
switch (mode) {
case "entry-pages":
@ -168,7 +170,7 @@ export default function Pages(props) {
<h3 className="font-bold dark:text-gray-100">
{labelFor[mode] || 'Page Visits'}
</h3>
<ImportedQueryUnsupportedWarning condition={importedQueryUnsupported}/>
<ImportedQueryUnsupportedWarning loading={loading} query={query} skipImportedReason={skipImportedReason} />
</div>
<div className="flex font-medium text-xs text-gray-500 dark:text-gray-400 space-x-2">
{renderPill('Top Pages', 'pages')}

View File

@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import * as api from '../../api'
import * as url from '../../util/url'
import { VISITORS_METRIC, maybeWithCR } from '../reports/metrics'
@ -6,16 +6,18 @@ import ListReport from '../reports/list'
import ImportedQueryUnsupportedWarning from '../../stats/imported-query-unsupported-warning'
export default function Referrers({source, site, query}) {
const [importedQueryUnsupported, setImportedQueryUnsupported] = useState(false)
const [skipImportedReason, setSkipImportedReason] = useState(null)
const [loading, setLoading] = useState(true)
useEffect(() => setLoading(true), [query])
function fetchReferrers() {
return api.get(url.apiPath(site, `/referrers/${encodeURIComponent(source)}`), query, {limit: 9})
}
function afterFetchReferrers(apiResponse) {
const unsupportedQuery = apiResponse.skip_imported_reason === 'unsupported_query'
const isRealtime = query.period === 'realtime'
setImportedQueryUnsupported(unsupportedQuery && !isRealtime)
setLoading(false)
setSkipImportedReason(apiResponse.skip_imported_reason)
}
function externalLinkDest(referrer) {
@ -46,7 +48,7 @@ export default function Referrers({source, site, query}) {
<div className="flex flex-col flex-grow">
<div className="flex gap-x-1">
<h3 className="font-bold dark:text-gray-100">Top Referrers</h3>
<ImportedQueryUnsupportedWarning condition={importedQueryUnsupported}/>
<ImportedQueryUnsupportedWarning loading={loading} query={query} skipImportedReason={skipImportedReason}/>
</div>
<ListReport
fetchData={fetchReferrers}

View File

@ -1,4 +1,4 @@
import React, { Fragment, useState } from 'react';
import React, { Fragment, useEffect, useState } from 'react';
import * as storage from '../../util/storage'
import * as url from '../../util/url'
@ -90,7 +90,10 @@ export default function SourceList(props) {
const tabKey = 'sourceTab__' + props.site.domain
const storedTab = storage.getItem(tabKey)
const [currentTab, setCurrentTab] = useState(storedTab || 'all')
const [importedQueryUnsupported, setImportedQueryUnsupported] = useState(false)
const [loading, setLoading] = useState(true)
const [skipImportedReason, setSkipImportedReason] = useState(null)
useEffect(() => setLoading(true), [query, currentTab])
function setTab(tab) {
return () => {
@ -163,9 +166,8 @@ export default function SourceList(props) {
}
function afterFetchData(apiResponse) {
const unsupportedQuery = apiResponse.skip_imported_reason === 'unsupported_query'
const isRealtime = query.period === 'realtime'
setImportedQueryUnsupported(unsupportedQuery && !isRealtime)
setLoading(false)
setSkipImportedReason(apiResponse.skip_imported_reason)
}
return (
@ -176,7 +178,7 @@ export default function SourceList(props) {
<h3 className="font-bold dark:text-gray-100">
Top Sources
</h3>
<ImportedQueryUnsupportedWarning condition={importedQueryUnsupported}/>
<ImportedQueryUnsupportedWarning loading={loading} query={query} skipImportedReason={skipImportedReason}/>
</div>
{renderTabs()}
</div>

View File

@ -169,7 +169,8 @@ defmodule Plausible.Stats.Comparisons do
:ok ->
struct!(query,
imported_data_requested: true,
include_imported: true
include_imported: true,
skip_imported_reason: nil
)
{:error, reason} ->

View File

@ -301,14 +301,14 @@ defmodule Plausible.Stats.Query do
end
@spec ensure_include_imported(t(), boolean()) ::
:ok | {:error, :not_requested | :no_imported_data | :out_of_range | :unsupported_query}
:ok | {:error, :no_imported_data | :out_of_range | :unsupported_query | :not_requested}
def ensure_include_imported(query, requested?) do
cond do
not requested? -> {:error, :not_requested}
is_nil(query.latest_import_end_date) -> {:error, :no_imported_data}
Date.after?(query.date_range.first, query.latest_import_end_date) -> {:error, :out_of_range}
not Imported.schema_supports_query?(query) -> {:error, :unsupported_query}
query.period == "realtime" -> {:error, :unsupported_query}
not requested? -> {:error, :not_requested}
true -> :ok
end
end

View File

@ -217,6 +217,7 @@ defmodule PlausibleWeb.Api.StatsController do
top_stats: top_stats,
interval: query.interval,
sample_percent: sample_percent,
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 && comparison_query.date_range.first,
@ -226,6 +227,38 @@ defmodule PlausibleWeb.Api.StatsController do
})
end
defp with_imported_switch_info(%Query{period: "realtime"}, _) do
%{visible: false, togglable: false, tooltip_msg: nil}
end
defp with_imported_switch_info(query, nil) do
with_imported_switch_info(query.skip_imported_reason)
end
defp with_imported_switch_info(query, comparison_query) do
case {query.skip_imported_reason, comparison_query.skip_imported_reason} do
{:out_of_range, nil} -> with_imported_switch_info(nil)
{:out_of_range, :not_requested} -> with_imported_switch_info(:not_requested)
{reason, _} -> with_imported_switch_info(reason)
end
end
defp with_imported_switch_info(skip_reason) do
case skip_reason do
reason when reason in [:no_imported_data, :out_of_range] ->
%{visible: false, togglable: false, tooltip_msg: nil}
:unsupported_query ->
%{visible: true, togglable: false, tooltip_msg: "Imported data cannot be included"}
:not_requested ->
%{visible: true, togglable: true, tooltip_msg: "Click to include imported data"}
nil ->
%{visible: true, togglable: true, tooltip_msg: "Click to exclude imported data"}
end
end
defp present_index_for(site, query, dates) do
case query.interval do
"hour" ->

View File

@ -628,6 +628,139 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do
end
end
describe "GET /api/stats/top-stats - with_imported_switch info" do
setup [:create_user, :log_in, :create_new_site]
setup context do
insert(:site_import,
site: context.site,
start_date: ~D[2021-03-01],
end_date: ~D[2021-03-31]
)
context
end
test "visible is false in realtime", %{conn: conn, site: site} do
q = "?period=realtime"
conn = get(conn, "/api/stats/#{site.domain}/top-stats#{q}")
res = json_response(conn, 200)
assert res["with_imported_switch"] == %{
"visible" => false,
"togglable" => false,
"tooltip_msg" => nil
}
end
test "when the site has no imported data", %{conn: conn, site: site} do
Plausible.Imported.delete_imports_for_site(site)
q = "?period=day&date=2021-01-01&with_imported=true"
conn = get(conn, "/api/stats/#{site.domain}/top-stats#{q}")
res = json_response(conn, 200)
assert res["with_imported_switch"] == %{
"visible" => false,
"togglable" => false,
"tooltip_msg" => nil
}
end
test "when imported data does not exist in the queried period", %{conn: conn, site: site} do
q = "?period=day&date=2022-05-05&with_imported=true"
conn = get(conn, "/api/stats/#{site.domain}/top-stats#{q}")
res = json_response(conn, 200)
assert res["with_imported_switch"] == %{
"visible" => false,
"togglable" => false,
"tooltip_msg" => nil
}
end
test "when imported data is requested, in range, and can be included", %{
conn: conn,
site: site
} do
q = "?period=day&date=2021-03-15&with_imported=true"
conn = get(conn, "/api/stats/#{site.domain}/top-stats#{q}")
res = json_response(conn, 200)
assert res["with_imported_switch"] == %{
"visible" => true,
"togglable" => true,
"tooltip_msg" => "Click to exclude imported data"
}
end
test "when imported data is not requested, but in range and can be included", %{
conn: conn,
site: site
} do
q = "?period=day&date=2021-03-15&with_imported=false"
conn = get(conn, "/api/stats/#{site.domain}/top-stats#{q}")
res = json_response(conn, 200)
assert res["with_imported_switch"] == %{
"visible" => true,
"togglable" => true,
"tooltip_msg" => "Click to include imported data"
}
end
test "when imported data is requested and in range, but cannot be included", %{
conn: conn,
site: site
} do
filters = Jason.encode!(%{goal: "Signup", page: "/register"})
q = "?period=day&date=2021-03-15&with_imported=true&filters=#{filters}"
conn = get(conn, "/api/stats/#{site.domain}/top-stats#{q}")
res = json_response(conn, 200)
assert res["with_imported_switch"] == %{
"visible" => true,
"togglable" => false,
"tooltip_msg" => "Imported data cannot be included"
}
end
test "when imported data is requested and in comparison range", %{conn: conn, site: site} do
q = "?period=day&date=2022-03-15&with_imported=true&comparison=year_over_year"
conn = get(conn, "/api/stats/#{site.domain}/top-stats#{q}")
res = json_response(conn, 200)
assert res["with_imported_switch"] == %{
"visible" => true,
"togglable" => true,
"tooltip_msg" => "Click to exclude imported data"
}
end
test "when imported data is not requested and in comparison range", %{conn: conn, site: site} do
q = "?period=day&date=2022-03-15&with_imported=false&comparison=year_over_year"
conn = get(conn, "/api/stats/#{site.domain}/top-stats#{q}")
res = json_response(conn, 200)
assert res["with_imported_switch"] == %{
"visible" => true,
"togglable" => true,
"tooltip_msg" => "Click to include imported data"
}
end
end
describe "GET /api/stats/top-stats - realtime" do
setup [:create_user, :log_in, :create_new_site]