Skip to content

Commit 60a47f3

Browse files
macobomacobo
authored
Fix: time series trimming (#5705)
* Fix time series trimming #5699 accidentally broke time series trimming on main graph It tried to remove a seemingly redundant Query.optimize call, but this was implicitly needed for time labels functionality. * Add testing tools, test timeseries better Tool lifted from #5680 * Minor cleanup
1 parent 6a9fe45 commit 60a47f3

6 files changed

Lines changed: 209 additions & 11 deletions

File tree

‎lib/plausible/stats/filters/query_parser.ex‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,8 @@ defmodule Plausible.Stats.Filters.QueryParser do
2727
def default_include(), do: @default_include
2828

2929
def parse(site, schema_type, params, now \\ nil) when is_map(params) do
30-
{now, date} =
31-
if now do
32-
{now, DateTime.shift_zone!(now, site.timezone) |> DateTime.to_date()}
33-
else
34-
{DateTime.utc_now(:second), today(site)}
35-
end
30+
now = now || Plausible.Stats.Query.Test.get_fixed_now()
31+
date = now |> DateTime.shift_zone!(site.timezone) |> DateTime.to_date()
3632

3733
with :ok <- JSONSchema.validate(schema_type, params),
3834
{:ok, date, now} <- parse_date(site, Map.get(params, "date"), date, now),
@@ -289,8 +285,6 @@ defmodule Plausible.Stats.Filters.QueryParser do
289285
end
290286
end
291287

292-
defp today(site), do: DateTime.now!(site.timezone) |> DateTime.to_date()
293-
294288
defp parse_dimensions(dimensions) when is_list(dimensions) do
295289
parse_list(
296290
dimensions,

‎lib/plausible/stats/legacy/legacy_query_builder.ex‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do
1111
alias Plausible.Stats.{Filters, Interval, Query, DateTimeRange}
1212

1313
def from(site, params, debug_metadata, now \\ nil) do
14-
now = now || DateTime.utc_now(:second)
14+
now = now || Plausible.Stats.Query.Test.get_fixed_now()
1515

1616
query =
1717
Query

‎lib/plausible/stats/query/test.ex‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
defmodule Plausible.Stats.Query.Test do
2+
@moduledoc """
3+
Module used in tests to 'set' the current time.
4+
"""
5+
6+
@now_key :__now
7+
8+
def fix_now(now) do
9+
Process.put(@now_key, now)
10+
end
11+
12+
def get_fixed_now() do
13+
Process.get(@now_key) || DateTime.utc_now(:second)
14+
end
15+
end

‎lib/plausible/stats/timeseries.ex‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ defmodule Plausible.Stats.Timeseries do
77

88
use Plausible
99
use Plausible.ClickhouseRepo
10-
alias Plausible.Stats.{Comparisons, Query, QueryRunner, Metrics, Time}
10+
alias Plausible.Stats.{Comparisons, Query, QueryRunner, Metrics, Time, QueryOptimizer}
1111

1212
@time_dimension %{
1313
"month" => "time:month",
@@ -26,6 +26,7 @@ defmodule Plausible.Stats.Timeseries do
2626
order_by: [{time_dimension(query), :asc}],
2727
remove_unavailable_revenue_metrics: true
2828
)
29+
|> QueryOptimizer.optimize()
2930

3031
comparison_query =
3132
if(query.include.comparisons,

‎test/plausible_web/controllers/api/external_stats_controller/query_comparisons_test.exs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,13 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryComparisonsTest do
185185
build(:pageview, timestamp: ~N[2022-07-01 00:00:00])
186186
])
187187

188+
Plausible.Stats.Query.Test.fix_now(~U[2022-07-01 14:00:00Z])
189+
188190
conn =
189191
post(conn, "/api/v2/query-internal-test", %{
190192
"site_id" => site.domain,
191193
"metrics" => ["visitors"],
192194
"date_range" => "91d",
193-
"date" => "2022-07-01",
194195
"dimensions" => ["time:day"],
195196
"include" => %{
196197
"time_labels" => true,

‎test/plausible_web/controllers/api/stats_controller/main_graph_test.exs‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,6 +1340,95 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
13401340

13411341
assert Enum.at(plot, Enum.find_index(labels, &(&1 == "2023-03-01 12:00:00"))) == 1
13421342
end
1343+
1344+
test "trims hourly relative date range", %{conn: conn, site: site} do
1345+
populate_stats(site, [
1346+
build(:pageview, timestamp: ~N[2021-01-08 00:00:00]),
1347+
build(:pageview, timestamp: ~N[2021-01-08 06:05:00]),
1348+
build(:pageview, timestamp: ~N[2021-01-08 08:59:00]),
1349+
build(:pageview, timestamp: ~N[2021-01-08 23:59:00])
1350+
])
1351+
1352+
Plausible.Stats.Query.Test.fix_now(~U[2021-01-08 08:05:00Z])
1353+
1354+
conn =
1355+
get(
1356+
conn,
1357+
"/api/stats/#{site.domain}/main-graph?period=day&metric=visitors&date=2021-01-08&interval=hour"
1358+
)
1359+
1360+
assert_matches %{
1361+
"labels" => [
1362+
"2021-01-08 00:00:00",
1363+
"2021-01-08 01:00:00",
1364+
"2021-01-08 02:00:00",
1365+
"2021-01-08 03:00:00",
1366+
"2021-01-08 04:00:00",
1367+
"2021-01-08 05:00:00",
1368+
"2021-01-08 06:00:00",
1369+
"2021-01-08 07:00:00",
1370+
"2021-01-08 08:00:00"
1371+
],
1372+
"plot" => [1, 0, 0, 0, 0, 0, 1, 0, 1]
1373+
} = json_response(conn, 200)
1374+
end
1375+
1376+
test "trims monthly relative date range", %{conn: conn, site: site} do
1377+
populate_stats(site, [
1378+
build(:pageview, timestamp: ~N[2021-01-01 00:00:00]),
1379+
build(:pageview, timestamp: ~N[2021-01-05 00:00:00]),
1380+
build(:pageview, timestamp: ~N[2021-01-07 00:00:00]),
1381+
build(:pageview, timestamp: ~N[2021-01-31 00:00:00])
1382+
])
1383+
1384+
Plausible.Stats.Query.Test.fix_now(~U[2021-01-07 12:00:00Z])
1385+
1386+
conn =
1387+
get(
1388+
conn,
1389+
"/api/stats/#{site.domain}/main-graph?period=month&metric=visitors&date=2021-01-07&interval=day"
1390+
)
1391+
1392+
assert_matches %{
1393+
"labels" => [
1394+
"2021-01-01",
1395+
"2021-01-02",
1396+
"2021-01-03",
1397+
"2021-01-04",
1398+
"2021-01-05",
1399+
"2021-01-06",
1400+
"2021-01-07"
1401+
],
1402+
"plot" => [1, 0, 0, 0, 1, 0, 1]
1403+
} = json_response(conn, 200)
1404+
end
1405+
1406+
test "trims yearly relative date range", %{conn: conn, site: site} do
1407+
populate_stats(site, [
1408+
build(:pageview, timestamp: ~N[2021-01-01 00:00:00]),
1409+
build(:pageview, timestamp: ~N[2021-01-05 00:00:00]),
1410+
build(:pageview, timestamp: ~N[2021-01-30 00:00:00]),
1411+
build(:pageview, timestamp: ~N[2021-01-31 00:00:00]),
1412+
build(:pageview, timestamp: ~N[2021-02-01 00:00:00]),
1413+
build(:pageview, timestamp: ~N[2021-02-09 00:00:00])
1414+
])
1415+
1416+
Plausible.Stats.Query.Test.fix_now(~U[2021-02-07 12:00:00Z])
1417+
1418+
conn =
1419+
get(
1420+
conn,
1421+
"/api/stats/#{site.domain}/main-graph?period=year&metric=visitors&date=2021-02-07&interval=month"
1422+
)
1423+
1424+
assert_matches %{
1425+
"labels" => [
1426+
"2021-01-01",
1427+
"2021-02-01"
1428+
],
1429+
"plot" => [4, 1]
1430+
} = json_response(conn, 200)
1431+
end
13431432
end
13441433

13451434
describe "GET /api/stats/main-graph - comparisons" do
@@ -1513,6 +1602,104 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
15131602
assert this_week_plot == [50.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0]
15141603
assert last_week_plot == [33.33, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0]
15151604
end
1605+
1606+
test "does not trim hourly relative date range when comparing", %{conn: conn, site: site} do
1607+
populate_stats(site, [
1608+
build(:pageview, timestamp: ~N[2021-01-08 00:00:00]),
1609+
build(:pageview, timestamp: ~N[2021-01-08 06:05:00]),
1610+
build(:pageview, timestamp: ~N[2021-01-08 08:59:00]),
1611+
build(:pageview, timestamp: ~N[2021-01-08 23:59:00])
1612+
])
1613+
1614+
Plausible.Stats.Query.Test.fix_now(~U[2021-01-08 08:05:00Z])
1615+
1616+
conn =
1617+
get(
1618+
conn,
1619+
"/api/stats/#{site.domain}/main-graph?period=day&metric=visitors&date=2021-01-08&interval=hour&comparison=previous_period"
1620+
)
1621+
1622+
assert_matches %{
1623+
"labels" => [
1624+
"2021-01-08 00:00:00",
1625+
"2021-01-08 01:00:00",
1626+
"2021-01-08 02:00:00",
1627+
"2021-01-08 03:00:00",
1628+
"2021-01-08 04:00:00",
1629+
"2021-01-08 05:00:00",
1630+
"2021-01-08 06:00:00",
1631+
"2021-01-08 07:00:00",
1632+
"2021-01-08 08:00:00",
1633+
"2021-01-08 09:00:00",
1634+
"2021-01-08 10:00:00",
1635+
"2021-01-08 11:00:00",
1636+
"2021-01-08 12:00:00",
1637+
"2021-01-08 13:00:00",
1638+
"2021-01-08 14:00:00",
1639+
"2021-01-08 15:00:00",
1640+
"2021-01-08 16:00:00",
1641+
"2021-01-08 17:00:00",
1642+
"2021-01-08 18:00:00",
1643+
"2021-01-08 19:00:00",
1644+
"2021-01-08 20:00:00",
1645+
"2021-01-08 21:00:00",
1646+
"2021-01-08 22:00:00",
1647+
"2021-01-08 23:00:00"
1648+
],
1649+
"plot" => [
1650+
1,
1651+
0,
1652+
0,
1653+
0,
1654+
0,
1655+
0,
1656+
1,
1657+
0,
1658+
1,
1659+
0,
1660+
0,
1661+
0,
1662+
0,
1663+
0,
1664+
0,
1665+
0,
1666+
0,
1667+
0,
1668+
0,
1669+
0,
1670+
0,
1671+
0,
1672+
0,
1673+
1
1674+
],
1675+
"comparison_plot" => [
1676+
0,
1677+
0,
1678+
0,
1679+
0,
1680+
0,
1681+
0,
1682+
0,
1683+
0,
1684+
0,
1685+
0,
1686+
0,
1687+
0,
1688+
0,
1689+
0,
1690+
0,
1691+
0,
1692+
0,
1693+
0,
1694+
0,
1695+
0,
1696+
0,
1697+
0,
1698+
0,
1699+
0
1700+
]
1701+
} = json_response(conn, 200)
1702+
end
15161703
end
15171704

15181705
describe "GET /api/stats/main-graph - total_revenue plot" do

0 commit comments

Comments
 (0)