Make comparisons to today consider only stats in the same exact time range (#6205)

* Make comparisons to today consider only stats in the same exact time range

* Update tests to account for date range behaviour adjustment

* Update CHANGELOG.md

* Refactor `get_comparison_datetime_range` (h/t @RobertJoonas)

Author: zoldar

CHANGELOG.md

@@ -13,6 +13,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
 

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,38 +132,34 @@ 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",
-           include: %{compare: :previous_period, compare_match_day_of_week: true}
+           input_date_range: input_range,
+           include: %{compare: :previous_period} = include
          } = source_query
-       ) 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)
-
-    DateTimeRange.new!(comparison_start, comparison_end)
-  end
+       )
+       when input_range in [:"24h", :day] do
+    offset =
+      if include.compare_match_day_of_week do
+        [day: -7]
+      else
+        [hour: -24]
+      end
 
-  defp get_comparison_datetime_range(
-         %Query{
-           input_date_range: :"24h",
-           include: %{compare: :previous_period}
-         } = source_query
-       ) 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)
+    comparison_start = DateTime.shift(source_query.utc_time_range.first, offset)
+    comparison_end = DateTime.shift(source_query.utc_time_range.last, offset)
 
     DateTimeRange.new!(comparison_start, comparison_end)
   end
 
   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)
 

lib/plausible/stats/datetime_range.ex

@@ -20,22 +20,26 @@ 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
         {:gap, _just_before, just_after} -> just_after
         {: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
         {:gap, just_before, _just_after} -> just_before
         {:ambiguous, first_datetime, _second_datetime} -> first_datetime
       end
 
-    new!(first, last)
+    new!(first, last, timezone)
   end
 
   def new!(%DateTime{} = first, %DateTime{} = last, _timezone) do

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

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

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}")

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
 

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},

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 =

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,