diff --git a/CHANGELOG.md b/CHANGELOG.md index fa4292529979..73a0892552a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file. - Adds team_id to query debug metadata (saved in system.query_log log_comment column) - Add "Unknown" option to Countries shield, for when the country code is unrecognized - Add "Last 24 Hours" to dashboard time range picker and Stats API v2 +- Always compare against the same time range in comparisons with "Today" ### Removed diff --git a/lib/plausible/stats/comparisons.ex b/lib/plausible/stats/comparisons.ex index 2321e17716b4..ca2ff68d7400 100644 --- a/lib/plausible/stats/comparisons.ex +++ b/lib/plausible/stats/comparisons.ex @@ -8,7 +8,8 @@ defmodule Plausible.Stats.Comparisons do """ alias Plausible.Stats - alias Plausible.Stats.{Query, DateTimeRange, Time} + alias Plausible.Stats.{DateTimeRange, Query, Time} + alias Plausible.Times @spec get_comparison_utc_time_range(Stats.Query.t()) :: DateTimeRange.t() @doc """ @@ -53,8 +54,8 @@ defmodule Plausible.Stats.Comparisons do DateTimeRange.new!(from, to) _ -> - # For 24h period, work directly with datetime ranges to preserve time precision - if source_query.input_date_range == :"24h" do + # For 24h period and today, work directly with datetime ranges to preserve time precision + if use_datetime_for_comparison?(source_query) do get_comparison_datetime_range(source_query) else comparison_date_range = get_comparison_date_range(source_query) @@ -70,6 +71,17 @@ defmodule Plausible.Stats.Comparisons do DateTimeRange.to_timezone(datetime_range, "Etc/UTC") end + defp use_datetime_for_comparison?(query) do + if query.input_date_range == :day do + today_from_now = Times.to_date(query.now, query.timezone) + day_from_range = Times.to_date(query.utc_time_range.first, query.timezone) + + Date.compare(today_from_now, day_from_range) == :eq + else + query.input_date_range == :"24h" + end + end + def get_comparison_query( %Query{comparison_utc_time_range: %DateTimeRange{} = comparison_range} = source_query ) do @@ -120,13 +132,14 @@ defmodule Plausible.Stats.Comparisons do end end - # For 24h period, shift the datetime range directly to preserve time precision + # For 24h and today periods, shift the datetime range directly to preserve time precision defp get_comparison_datetime_range( %Query{ - input_date_range: :"24h", + input_date_range: input_range, include: %{compare: :previous_period, compare_match_day_of_week: true} } = source_query - ) do + ) + when input_range in [:"24h", :day] do days_back = 7 comparison_start = DateTime.shift(source_query.utc_time_range.first, day: -days_back) comparison_end = DateTime.shift(source_query.utc_time_range.last, day: -days_back) @@ -136,10 +149,11 @@ defmodule Plausible.Stats.Comparisons do defp get_comparison_datetime_range( %Query{ - input_date_range: :"24h", + input_date_range: input_range, include: %{compare: :previous_period} } = source_query - ) do + ) + when input_range in [:"24h", :day] do comparison_start = DateTime.shift(source_query.utc_time_range.first, hour: -24) comparison_end = DateTime.shift(source_query.utc_time_range.last, hour: -24) @@ -148,10 +162,11 @@ defmodule Plausible.Stats.Comparisons do defp get_comparison_datetime_range( %Query{ - input_date_range: :"24h", + input_date_range: input_range, include: %{compare: :year_over_year} } = source_query - ) do + ) + when input_range in [:"24h", :day] do comparison_start = DateTime.shift(source_query.utc_time_range.first, year: -1) comparison_end = DateTime.shift(source_query.utc_time_range.last, year: -1) diff --git a/lib/plausible/stats/datetime_range.ex b/lib/plausible/stats/datetime_range.ex index b2fc6756bdce..a9584038910f 100644 --- a/lib/plausible/stats/datetime_range.ex +++ b/lib/plausible/stats/datetime_range.ex @@ -20,7 +20,7 @@ defmodule Plausible.Stats.DateTimeRange do will become the last date at 23:59:59. Both dates will be turned into `%DateTime{}` structs in the given timezone. """ - def new!(%Date{} = first, %Date{} = last, timezone) do + def new!(%Date{} = first, last, timezone) do first = case DateTime.new(first, ~T[00:00:00], timezone) do {:ok, datetime} -> datetime @@ -28,6 +28,10 @@ defmodule Plausible.Stats.DateTimeRange do {:ambiguous, _first_datetime, second_datetime} -> second_datetime end + new!(first, last, timezone) + end + + def new!(first, %Date{} = last, timezone) do last = case DateTime.new(last, ~T[23:59:59], timezone) do {:ok, datetime} -> datetime @@ -35,7 +39,7 @@ defmodule Plausible.Stats.DateTimeRange do {:ambiguous, first_datetime, _second_datetime} -> first_datetime end - new!(first, last) + new!(first, last, timezone) end def new!(%DateTime{} = first, %DateTime{} = last, _timezone) do diff --git a/lib/plausible/stats/legacy/legacy_query_builder.ex b/lib/plausible/stats/legacy/legacy_query_builder.ex index 03bc4b917acd..aa615210807d 100644 --- a/lib/plausible/stats/legacy/legacy_query_builder.ex +++ b/lib/plausible/stats/legacy/legacy_query_builder.ex @@ -9,6 +9,7 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do use Plausible alias Plausible.Stats.{Filters, Interval, Query, ApiQueryParser, QueryBuilder, DateTimeRange} + alias Plausible.Times def from(site, params, debug_metadata, now \\ nil) do now = now || DateTime.utc_now(:second) @@ -99,11 +100,16 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do struct!(query, input_date_range: input_date_range_atom, utc_time_range: datetime_range) end - defp put_input_date_range(query, site, %{"period" => "day"} = params) do + defp put_input_date_range(%Query{now: now} = query, site, %{"period" => "day"} = params) do date = parse_single_date(query, params) datetime_range = - DateTimeRange.new!(date, date, site.timezone) |> DateTimeRange.to_timezone("Etc/UTC") + if Date.compare(Times.to_date(now, site.timezone), date) == :eq do + DateTimeRange.new!(date, now, site.timezone) + else + DateTimeRange.new!(date, date, site.timezone) + end + |> DateTimeRange.to_timezone("Etc/UTC") struct!(query, input_date_range: :day, utc_time_range: datetime_range) end diff --git a/lib/plausible/stats/query_builder.ex b/lib/plausible/stats/query_builder.ex index 1d9f6e137ad4..d739b9bb0e88 100644 --- a/lib/plausible/stats/query_builder.ex +++ b/lib/plausible/stats/query_builder.ex @@ -18,6 +18,8 @@ defmodule Plausible.Stats.QueryBuilder do QueryInclude } + alias Plausible.Times + @doc """ Runs various validations and builds a `%Query{}` from already parsed params. @@ -96,8 +98,12 @@ defmodule Plausible.Stats.QueryBuilder do DateTimeRange.new!(first_datetime, last_datetime) end - defp build_datetime_range(:day, site, relative_date, _now) do - DateTimeRange.new!(relative_date, relative_date, site.timezone) + defp build_datetime_range(:day, site, relative_date, now) do + if Date.compare(Times.to_date(now, site.timezone), relative_date) == :eq do + DateTimeRange.new!(relative_date, now, site.timezone) + else + DateTimeRange.new!(relative_date, relative_date, site.timezone) + end end defp build_datetime_range(:"24h", _site, _relative_date, now) do @@ -153,7 +159,7 @@ defmodule Plausible.Stats.QueryBuilder do dimensions: dimensions } = parsed_query_params - relative_date = relative_date || today_from_now(site, now) + relative_date = relative_date || Times.to_date(now, site.timezone) utc_time_range = input_date_range @@ -560,10 +566,6 @@ defmodule Plausible.Stats.QueryBuilder do end end - defp today_from_now(site, now) do - now |> DateTime.shift_zone!(site.timezone) |> DateTime.to_date() - end - defp i(value), do: inspect(value, charlists: :as_lists) defp validate_list(list, parser_function) do diff --git a/lib/plausible/times.ex b/lib/plausible/times.ex index 345b380ad55b..aa803bc6bffd 100644 --- a/lib/plausible/times.ex +++ b/lib/plausible/times.ex @@ -62,6 +62,13 @@ defmodule Plausible.Times do Timex.end_of_year(t) end + @spec to_date(DateTime.t(), String.t()) :: Date.t() + def to_date(%DateTime{} = dt, tz) do + dt + |> DateTime.shift_zone!(tz) + |> DateTime.to_date() + end + @spec humanize(DateTime.t()) :: String.t() def humanize(%DateTime{} = dt) do Timex.Format.DateTime.Formatters.Relative.format!(dt, "{relative}") diff --git a/test/plausible/stats/query/query_from_test.exs b/test/plausible/stats/query/query_from_test.exs index a5546f0b0880..e03a4ff9665a 100644 --- a/test/plausible/stats/query/query_from_test.exs +++ b/test/plausible/stats/query/query_from_test.exs @@ -47,7 +47,7 @@ defmodule Plausible.Stats.Query.QueryFromTest do q = Query.from(site, %{"period" => "day"}, now: @now) assert q.utc_time_range.first == ~U[2024-05-03 04:00:00Z] - assert q.utc_time_range.last == ~U[2024-05-04 03:59:59Z] + assert q.utc_time_range.last == @now assert q.interval == "hour" end diff --git a/test/plausible/stats/query/query_parse_and_build_test.exs b/test/plausible/stats/query/query_parse_and_build_test.exs index eb4320b220db..33add7a73963 100644 --- a/test/plausible/stats/query/query_parse_and_build_test.exs +++ b/test/plausible/stats/query/query_parse_and_build_test.exs @@ -5,6 +5,10 @@ defmodule Plausible.Stats.Query.QueryParseAndBuildTest do alias Plausible.Stats.{Query, DateTimeRange, Filters, QueryError} @now DateTime.new!(~D[2021-05-05], ~T[12:30:00], "Etc/UTC") + @date_range_today %DateTimeRange{ + first: DateTime.new!(~D[2021-05-05], ~T[00:00:00], "Etc/UTC"), + last: @now + } @date_range_day %DateTimeRange{ first: DateTime.new!(~D[2021-05-05], ~T[00:00:00], "Etc/UTC"), last: DateTime.new!(~D[2021-05-05], ~T[23:59:59], "Etc/UTC") @@ -1217,7 +1221,7 @@ defmodule Plausible.Stats.Query.QueryParseAndBuildTest do end for {shortcut, expected_date_range} <- [ - {"day", @date_range_day}, + {"day", @date_range_today}, {"7d", @date_range_7d}, {"10d", @date_range_10d}, {"30d", @date_range_30d}, diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_timezone_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_timezone_test.exs index 671afcdce862..32c69ff0de49 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_timezone_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_timezone_test.exs @@ -41,7 +41,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTimezoneTest do populate_stats(site, [ build(:pageview, timestamp: ~N[2024-01-01 10:00:00]), build(:pageview, timestamp: ~N[2024-01-01 12:00:00]), - build(:pageview, timestamp: ~N[2024-01-02 12:00:00]) + build(:pageview, timestamp: ~N[2024-01-01 15:30:00]) ]) conn = diff --git a/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs b/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs index 214ee469c5b4..8a22db61af3b 100644 --- a/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs @@ -1583,12 +1583,11 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do assert last_week_plot == [33.33, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0] end - test "does not trim hourly relative date range when comparing", %{conn: conn, site: site} do + test "does trim hourly relative date range when comparing", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, timestamp: ~N[2021-01-08 00:00:00]), build(:pageview, timestamp: ~N[2021-01-08 06:05:00]), - build(:pageview, timestamp: ~N[2021-01-08 08:59:00]), - build(:pageview, timestamp: ~N[2021-01-08 23:59:00]) + build(:pageview, timestamp: ~N[2021-01-08 08:04:00]) ]) conn = @@ -1608,22 +1607,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do "2021-01-08 05:00:00", "2021-01-08 06:00:00", "2021-01-08 07:00:00", - "2021-01-08 08:00:00", - "2021-01-08 09:00:00", - "2021-01-08 10:00:00", - "2021-01-08 11:00:00", - "2021-01-08 12:00:00", - "2021-01-08 13:00:00", - "2021-01-08 14:00:00", - "2021-01-08 15:00:00", - "2021-01-08 16:00:00", - "2021-01-08 17:00:00", - "2021-01-08 18:00:00", - "2021-01-08 19:00:00", - "2021-01-08 20:00:00", - "2021-01-08 21:00:00", - "2021-01-08 22:00:00", - "2021-01-08 23:00:00" + "2021-01-08 08:00:00" ], "plot" => [ 1, @@ -1634,39 +1618,9 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do 0, 1, 0, - 1, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, 1 ], "comparison_plot" => [ - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, - 0, 0, 0, 0,