Disallow small intervals on large ranges (#3245)

* Update interval dropdown based on custom range selection

* Add `monthsBetweenDates` function

* Reassign interval for custom "period" based on local storage

* Generate intervals by period based on stats_start_date

* Account for "custom" and "all" period intervals dynamically
This commit is contained in:
hq1 2023-08-08 14:49:19 +02:00 committed by GitHub
parent eeca63ff4c
commit e467324c5a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 242 additions and 90 deletions

View File

@ -4,6 +4,7 @@ import React, { Fragment, useCallback, useEffect } from 'react';
import classNames from 'classnames'
import * as storage from '../../util/storage'
import { isKeyPressed } from '../../keybinding.js'
import { monthsBetweenDates } from '../../util/date.js'
export const INTERVAL_LABELS = {
'minute': 'Minutes',
@ -37,11 +38,11 @@ function DropdownItem({ option, currentInterval, updateInterval }) {
<Menu.Item onClick={() => updateInterval(option)} key={option} disabled={option == currentInterval}>
{({ active }) => (
<span className={classNames({
'bg-gray-100 dark:bg-gray-900 text-gray-900 dark:text-gray-200 cursor-pointer': active,
'text-gray-700 dark:text-gray-200': !active,
'font-bold cursor-none select-none': option == currentInterval,
}, 'block px-4 py-2 text-sm')}>
{ INTERVAL_LABELS[option] }
'bg-gray-100 dark:bg-gray-900 text-gray-900 dark:text-gray-200 cursor-pointer': active,
'text-gray-700 dark:text-gray-200': !active,
'font-bold cursor-none select-none': option == currentInterval,
}, 'block px-4 py-2 text-sm')}>
{INTERVAL_LABELS[option]}
</span>
)}
</Menu.Item>
@ -54,15 +55,23 @@ export function IntervalPicker({ graphData, query, site, updateInterval }) {
const menuElement = React.useRef(null)
subscribeKeybinding(menuElement)
const currentInterval = graphData?.interval
const options = site.validIntervalsByPeriod[query.period]
let currentInterval = graphData?.interval
let options = site.validIntervalsByPeriod[query.period]
if (query.period === "custom" && monthsBetweenDates(query.from, query.to) > 12) {
options = ["week", "month"]
}
if (!options.includes(currentInterval)) {
currentInterval = [...options].pop()
}
return (
<Menu as="div" className="relative inline-block pl-2">
{({ open }) => (
<>
<Menu.Button ref={menuElement} className="text-sm inline-flex focus:outline-none text-gray-700 dark:text-gray-300 hover:text-indigo-600 dark:hover:text-indigo-600 items-center">
{ INTERVAL_LABELS[currentInterval] }
{INTERVAL_LABELS[currentInterval]}
<ChevronDownIcon className="ml-1 h-4 w-4" aria-hidden="true" />
</Menu.Button>
@ -76,7 +85,7 @@ export function IntervalPicker({ graphData, query, site, updateInterval }) {
leaveFrom="transform opacity-100 scale-100"
leaveTo="transform opacity-0 scale-95">
<Menu.Items className="py-1 text-left origin-top-right absolute right-0 mt-2 w-56 rounded-md shadow-lg bg-white dark:bg-gray-800 ring-1 ring-black ring-opacity-5 focus:outline-none z-10" static>
{ options.map((option) => DropdownItem({ option, currentInterval, updateInterval })) }
{options.map((option) => DropdownItem({ option, currentInterval, updateInterval }))}
</Menu.Items>
</Transition>
</>

View File

@ -13,7 +13,7 @@ import { IntervalPicker, getStoredInterval, storeInterval } from './interval-pic
import FadeIn from '../../fade-in';
import * as url from '../../util/url'
import classNames from 'classnames';
import { parseNaiveDate, isBefore } from '../../util/date'
import { monthsBetweenDates, parseNaiveDate, isBefore } from '../../util/date'
import { isComparisonEnabled } from '../../comparison-input'
const calculateMaximumY = function(dataset) {
@ -33,7 +33,7 @@ class LineGraph extends React.Component {
super(props);
this.boundary = React.createRef()
this.regenerateChart = this.regenerateChart.bind(this);
this.updateWindowDimensions = this.updateWindowDimensions.bind(this);
this.updateWindowDimensions = this.updateWindowDimensions.bind(this);
this.state = {
exported: false
};
@ -93,15 +93,15 @@ class LineGraph extends React.Component {
x: {
grid: { display: false },
ticks: {
callback: function (val, _index, _ticks) {
callback: function(val, _index, _ticks) {
if (this.getLabelForValue(val) == "__blank__") return ""
const hasMultipleYears =
graphData.labels
.filter((date) => typeof date === 'string')
.map(date => date.split('-')[0])
.filter((value, index, list) => list.indexOf(value) === index)
.length > 1
.filter((date) => typeof date === 'string')
.map(date => date.split('-')[0])
.filter((value, index, list) => list.indexOf(value) === index)
.length > 1
if (graphData.interval === 'hour' && query.period !== 'day') {
const date = dateFormatter({
@ -235,12 +235,12 @@ class LineGraph extends React.Component {
if (document.cookie.includes('exporting')) {
setTimeout(this.pollExportReady.bind(this), 1000);
} else {
this.setState({exported: false})
this.setState({ exported: false })
}
}
downloadSpinner() {
this.setState({exported: true});
this.setState({ exported: true });
document.cookie = "exporting=";
setTimeout(this.pollExportReady.bind(this), 1000);
}
@ -304,14 +304,14 @@ class LineGraph extends React.Component {
const source = this.props.topStatData.imported_source
const withImported = this.props.topStatData.with_imported;
const strike = withImported ? "" : " line-through"
const target = url.setQuery('with_imported', !withImported)
const target = url.setQuery('with_imported', !withImported)
const tip = withImported ? "" : "do not ";
return (
<Link to={target} className="w-4 h-4 mx-2">
<div tooltip={`Stats ${tip}include data imported from ${source}.`} className="cursor-pointer w-4 h-4">
<svg className="absolute dark:text-gray-300 text-gray-700" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor">
<text x="4" y="18" fontSize="24" fill="currentColor" className={"text-gray-700 dark:text-gray-300" + strike}>{ source[0].toUpperCase() }</text>
<text x="4" y="18" fontSize="24" fill="currentColor" className={"text-gray-700 dark:text-gray-300" + strike}>{source[0].toUpperCase()}</text>
</svg>
</div>
</Link>
@ -333,20 +333,20 @@ class LineGraph extends React.Component {
render() {
const { mainGraphRefreshing, updateMetric, updateInterval, metric, topStatData, query, site, graphData, lastLoadTimestamp } = this.props
const canvasClass = classNames('mt-4 select-none', {'cursor-pointer': !['minute', 'hour'].includes(graphData?.interval)})
const canvasClass = classNames('mt-4 select-none', { 'cursor-pointer': !['minute', 'hour'].includes(graphData?.interval) })
return (
<div>
<div id="top-stats-container" className="flex flex-wrap" ref={this.boundary} style={{height: this.getTopStatsHeight()}}>
<div id="top-stats-container" className="flex flex-wrap" ref={this.boundary} style={{ height: this.getTopStatsHeight() }}>
<TopStats site={site} query={query} metric={metric} updateMetric={updateMetric} topStatData={topStatData} tooltipBoundary={this.boundary.current} lastLoadTimestamp={lastLoadTimestamp} />
</div>
<div className="relative px-2">
{mainGraphRefreshing && renderLoader()}
<div className="absolute right-4 -top-8 py-1 flex items-center">
{ this.downloadLink() }
{ this.samplingNotice() }
{ this.importedNotice() }
<IntervalPicker site={site} query={query} graphData={graphData} metric={metric} updateInterval={updateInterval}/>
{this.downloadLink()}
{this.samplingNotice()}
{this.importedNotice()}
<IntervalPicker site={site} query={query} graphData={graphData} metric={metric} updateInterval={updateInterval} />
</div>
<FadeIn show={graphData}>
<div className="relative h-96 w-full z-0">
@ -385,10 +385,14 @@ export default class VisitorGraph extends React.Component {
getIntervalFromStorage() {
const { query, site } = this.props
const storedInterval = getStoredInterval(query.period, site.domain)
let interval = getStoredInterval(query.period, site.domain)
if (this.isIntervalValid(storedInterval)) {
return storedInterval
if (interval !== "week" && interval !== "month" && query.period === "custom" && monthsBetweenDates(query.from, query.to) > 12) {
interval = "month"
}
if (this.isIntervalValid(interval)) {
return interval
} else {
return null
}
@ -402,7 +406,7 @@ export default class VisitorGraph extends React.Component {
}
onVisible() {
this.setState({mainGraphLoadingState: LoadingState.loading}, this.fetchGraphData)
this.setState({ mainGraphLoadingState: LoadingState.loading }, this.fetchGraphData)
this.fetchTopStatData()
if (this.props.query.period === 'realtime') {
document.addEventListener('tick', this.fetchGraphData)
@ -420,7 +424,7 @@ export default class VisitorGraph extends React.Component {
}
if (metric !== prevState.metric) {
this.setState({mainGraphLoadingState: LoadingState.refreshing}, this.fetchGraphData)
this.setState({ mainGraphLoadingState: LoadingState.refreshing }, this.fetchGraphData)
}
}
@ -510,7 +514,7 @@ export default class VisitorGraph extends React.Component {
}
render() {
const {mainGraphLoadingState, topStatsLoadingState} = this.state
const { mainGraphLoadingState, topStatsLoadingState } = this.state
const showLoader =
[mainGraphLoadingState, topStatsLoadingState].includes(LoadingState.loading) &&

View File

@ -124,3 +124,8 @@ export function isAfter(date1, date2, period) {
}
return date1.date() > date2.date()
}
export function monthsBetweenDates(date1, date2) {
var diffMonths = (date2.year() - date1.year()) * 12 + date2.month() - date1.month();
return Math.abs(diffMonths);
}

View File

@ -8,6 +8,8 @@ defmodule Plausible.Stats.Interval do
"""
@type t() :: String.t()
@type(opt() :: {:site, Plausible.Site.t()} | {:from, Date.t()}, {:to, Date.t()})
@type opts :: list(opt())
@typep period() :: String.t()
@intervals ~w(minute hour date week month)
@ -22,12 +24,6 @@ defmodule Plausible.Stats.Interval do
@spec default_for_period(period()) :: t()
@doc """
Returns the suggested interval for the given time period.
## Examples
iex> Plausible.Stats.Interval.default_for_period("7d")
"date"
"""
def default_for_period(period) do
case period do
@ -38,21 +34,10 @@ defmodule Plausible.Stats.Interval do
end
end
@spec default_for_date_range(Date.Range.t()) :: t()
@doc """
Returns the suggested interval for the given `Date.Range` struct.
## Examples
iex> Plausible.Stats.Interval.default_for_date_range(Date.range(~D[2022-01-01], ~D[2023-01-01]))
"month"
iex> Plausible.Stats.Interval.default_for_date_range(Date.range(~D[2022-01-01], ~D[2022-01-15]))
"date"
iex> Plausible.Stats.Interval.default_for_date_range(Date.range(~D[2022-01-01], ~D[2022-01-01]))
"hour"
"""
def default_for_date_range(%Date.Range{first: first, last: last}) do
cond do
Timex.diff(last, first, :months) > 0 ->
@ -78,30 +63,44 @@ defmodule Plausible.Stats.Interval do
"custom" => ["date", "week", "month"],
"all" => ["date", "week", "month"]
}
def valid_by_period, do: @valid_by_period
@spec valid_for_period?(period(), t()) :: boolean()
@spec valid_by_period(opts()) :: map()
def valid_by_period(opts \\ []) do
site = Keyword.fetch!(opts, :site)
table =
with %Date{} = from <- Keyword.get(opts, :from),
%Date{} = to <- Keyword.get(opts, :to),
true <- abs(Timex.diff(from, to, :months)) > 12 do
Map.replace(@valid_by_period, "custom", ["week", "month"])
else
_ ->
@valid_by_period
end
with %Date{} = stats_start <- site.stats_start_date,
true <- abs(Timex.diff(Date.utc_today(), stats_start, :months)) > 12 do
Map.replace(table, "all", ["week", "month"])
else
_ ->
table
end
end
@spec valid_for_period?(period(), t(), opts()) :: boolean()
@doc """
Returns whether the given interval is valid for a time period.
Intervals longer than periods are not supported, e.g. current month stats with
a month interval, or today stats with a week interval.
## Examples
iex> Plausible.Stats.Interval.valid_for_period?("month", "date")
true
iex> Plausible.Stats.Interval.valid_for_period?("30d", "month")
false
iex> Plausible.Stats.Interval.valid_for_period?("realtime", "week")
false
There are two dynamic states:
* `custom` period is only applicable with `month` or `week` intervals,
if the `opts[:from]` and `opts[:to]` range difference exceeds 12 months
* `all` period's interval options depend on particular site's `stats_start_date`
- daily interval is excluded if the all-time range exceeds 12 months
"""
def valid_for_period?(period, interval) do
allowed = Map.get(@valid_by_period, period, [])
interval in allowed
def valid_for_period?(period, interval, opts \\ []) do
interval in Map.get(valid_by_period(opts), period, [])
end
end

View File

@ -94,7 +94,7 @@ defmodule PlausibleWeb.Api.StatsController do
def main_graph(conn, params) do
site = conn.assigns[:site]
with :ok <- validate_params(params) do
with :ok <- validate_params(site, params) do
query = Query.from(site, params) |> Filters.add_prefix()
selected_metric =
@ -199,7 +199,7 @@ defmodule PlausibleWeb.Api.StatsController do
def top_stats(conn, params) do
site = conn.assigns[:site]
with :ok <- validate_params(params) do
with :ok <- validate_params(site, params) do
query = Query.from(site, params) |> Filters.add_prefix()
comparison_opts = parse_comparison_opts(params)
@ -515,7 +515,7 @@ defmodule PlausibleWeb.Api.StatsController do
def funnel(conn, %{"id" => funnel_id} = params) do
site = conn.assigns[:site]
with :ok <- validate_params(params),
with :ok <- validate_params(site, params),
query <- Query.from(site, params) |> Filters.add_prefix(),
:ok <- validate_funnel_query(query),
{funnel_id, ""} <- Integer.parse(funnel_id),
@ -1444,25 +1444,25 @@ defmodule PlausibleWeb.Api.StatsController do
end
defp validate_common_input(conn, _opts) do
case validate_params(conn.params) do
case validate_params(conn.assigns[:site], conn.params) do
:ok -> conn
{:error, message} when is_binary(message) -> bad_request(conn, message)
end
end
defp validate_params(params) do
with :ok <- validate_dates(params),
defp validate_params(site, params) do
with {:ok, dates} <- validate_dates(params),
:ok <- validate_interval(params),
do: validate_interval_granularity(params)
do: validate_interval_granularity(site, params, dates)
end
defp validate_dates(params) do
params
|> Map.take(["from", "to", "date"])
|> Enum.reduce_while(:ok, fn {key, value}, _ ->
|> Enum.reduce_while({:ok, %{}}, fn {key, value}, {:ok, acc} ->
case Date.from_iso8601(value) do
{:ok, _} ->
{:cont, :ok}
{:ok, date} ->
{:cont, {:ok, Map.put(acc, key, date)}}
_ ->
{:halt,
@ -1486,17 +1486,30 @@ defmodule PlausibleWeb.Api.StatsController do
end
end
defp validate_interval_granularity(params) do
with %{"interval" => interval, "period" => period} <- params,
true <- Plausible.Stats.Interval.valid_for_period?(period, interval) do
:ok
else
%{} ->
:ok
defp validate_interval_granularity(site, params, dates) do
case params do
%{"interval" => interval, "period" => "custom", "from" => _, "to" => _} ->
if Plausible.Stats.Interval.valid_for_period?("custom", interval,
site: site,
from: dates["from"],
to: dates["to"]
) do
:ok
else
{:error,
"Invalid combination of interval and period. Custom ranges over 12 months must come with greater granularity, e.g. `period=custom,interval=week`"}
end
false ->
{:error,
"Invalid combination of interval and period. Interval must be smaller than the selected period, e.g. `period=day,interval=minute`"}
%{"interval" => interval, "period" => period} ->
if Plausible.Stats.Interval.valid_for_period?(period, interval, site: site) do
:ok
else
{:error,
"Invalid combination of interval and period. Interval must be smaller than the selected period, e.g. `period=day,interval=minute`"}
end
_ ->
:ok
end
end

View File

@ -32,7 +32,7 @@
data-is-dbip="<%= @is_dbip %>"
data-current-user-role="<%= @conn.assigns[:current_user_role] %>"
data-flags="<%= Jason.encode!(@flags) %>"
data-valid-intervals-by-period="<%= Plausible.Stats.Interval.valid_by_period() |> Jason.encode!() %>">
data-valid-intervals-by-period="<%= Plausible.Stats.Interval.valid_by_period(site: @site) |> Jason.encode!() %>">
</div>
<div id="modal_root"></div>
<%= if !@conn.assigns[:current_user] && @conn.assigns[:demo] do %>

View File

@ -1,4 +1,126 @@
defmodule Plausible.Stats.IntervalTest do
use ExUnit.Case, async: true
doctest Plausible.Stats.Interval
use Plausible.DataCase, async: true
import Plausible.Stats.Interval
test "default_for_period/1" do
assert default_for_period("realtime") == "minute"
assert default_for_period("day") == "hour"
assert default_for_period("7d") == "date"
assert default_for_period("12mo") == "month"
end
test "default_for_date_range/1" do
assert default_for_date_range(Date.range(~D[2022-01-01], ~D[2023-01-01])) == "month"
assert default_for_date_range(Date.range(~D[2022-01-01], ~D[2022-01-15])) == "date"
assert default_for_date_range(Date.range(~D[2022-01-01], ~D[2022-01-01])) == "hour"
end
describe "valid_by_period/1" do
test "for a newly created site" do
site = build(:site, stats_start_date: Date.utc_today())
assert valid_by_period(site: site) == %{
"realtime" => ["minute"],
"day" => ["minute", "hour"],
"7d" => ["hour", "date"],
"month" => ["date", "week"],
"30d" => ["date", "week"],
"6mo" => ["date", "week", "month"],
"12mo" => ["date", "week", "month"],
"year" => ["date", "week", "month"],
"custom" => ["date", "week", "month"],
"all" => ["date", "week", "month"]
}
end
test "for a site with stats starting over 12m ago" do
site = build(:site, stats_start_date: Timex.shift(Date.utc_today(), months: -13))
assert valid_by_period(site: site) == %{
"realtime" => ["minute"],
"day" => ["minute", "hour"],
"7d" => ["hour", "date"],
"month" => ["date", "week"],
"30d" => ["date", "week"],
"6mo" => ["date", "week", "month"],
"12mo" => ["date", "week", "month"],
"year" => ["date", "week", "month"],
"custom" => ["date", "week", "month"],
"all" => ["week", "month"]
}
end
test "for a query range exceeding 12m" do
ago_13m = Timex.shift(Date.utc_today(), months: -13)
site = build(:site, stats_start_date: ago_13m)
assert valid_by_period(site: site, from: ago_13m, to: Date.utc_today()) == %{
"realtime" => ["minute"],
"day" => ["minute", "hour"],
"7d" => ["hour", "date"],
"month" => ["date", "week"],
"30d" => ["date", "week"],
"6mo" => ["date", "week", "month"],
"12mo" => ["date", "week", "month"],
"year" => ["date", "week", "month"],
"custom" => ["week", "month"],
"all" => ["week", "month"]
}
end
end
describe "valid_for_period/3" do
test "common" do
site = build(:site)
assert valid_for_period?("month", "date", site: site)
refute valid_for_period?("30d", "month", site: site)
refute valid_for_period?("realtime", "week", site: site)
end
test "for a newly created site" do
site = build(:site, stats_start_date: Date.utc_today())
assert valid_for_period?("all", "date", site: site)
assert valid_for_period?("custom", "date",
site: site,
from: ~D[2023-06-01],
to: ~D[2023-07-01]
)
assert valid_for_period?("custom", "date",
site: site,
to: ~D[2023-06-01],
from: ~D[2023-07-01]
)
end
test "for a newly created site with >12m range" do
site = build(:site, stats_start_date: Date.utc_today())
assert valid_for_period?("all", "date", site: site)
refute valid_for_period?("custom", "date",
site: site,
from: ~D[2012-06-01],
to: ~D[2023-07-01]
)
refute valid_for_period?("custom", "date",
site: site,
to: ~D[2012-06-01],
from: ~D[2023-07-01]
)
end
test "for a site with stats starting over 12m ago" do
site = build(:site, stats_start_date: Timex.shift(Date.utc_today(), months: -13))
refute valid_for_period?("all", "date", site: site)
assert valid_for_period?("custom", "date",
site: site,
from: ~D[2023-06-01],
to: ~D[2023-07-01]
)
end
end
end