From 645b81376c04b4b130e44f2673745ac2d8ca85a6 Mon Sep 17 00:00:00 2001
From: Karl-Aksel Puulmann <macobo@users.noreply.github.com>
Date: Wed, 10 Jul 2024 15:46:31 +0300
Subject: [PATCH] fix(main-graph): fix 12mo and other comparisons with blanks
 (#4333)

Some comparisons return __blank__ values. After recent time series fix,
this blew up when trying to parse __blank__ as a date.

This fixes the issue and adds a test for a period with __blank__ values

Co-authored-by: RobertJoonas <56999674+RobertJoonas@users.noreply.github.com>
---
 .../controllers/api/stats_controller.ex       | 44 +++++++++----------
 .../api/stats_controller/main_graph_test.exs  | 30 +++++++++++++
 2 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex
index b3beef0e2..b308f43e0 100644
--- a/lib/plausible_web/controllers/api/stats_controller.ex
+++ b/lib/plausible_web/controllers/api/stats_controller.ex
@@ -172,36 +172,36 @@ defmodule PlausibleWeb.Api.StatsController do
     end
   end
 
-  defp build_full_intervals(%{interval: "week", date_range: range}, labels) do
-    for label <- labels, into: %{} do
-      date = Date.from_iso8601!(label)
-
-      interval_start = Timex.beginning_of_week(date)
-      interval_end = Timex.end_of_week(date)
-
-      within_interval? = Enum.member?(range, interval_start) && Enum.member?(range, interval_end)
-
-      {label, within_interval?}
-    end
+  defp build_full_intervals(%{interval: "week", date_range: date_range}, labels) do
+    build_intervals(labels, date_range, &Timex.beginning_of_week/1, &Timex.end_of_week/1)
   end
 
-  defp build_full_intervals(%{interval: "month", date_range: range}, labels) do
-    for label <- labels, into: %{} do
-      date = Date.from_iso8601!(label)
-
-      interval_start = Timex.beginning_of_month(date)
-      interval_end = Timex.end_of_month(date)
-
-      within_interval? = Enum.member?(range, interval_start) && Enum.member?(range, interval_end)
-
-      {label, within_interval?}
-    end
+  defp build_full_intervals(%{interval: "month", date_range: date_range}, labels) do
+    build_intervals(labels, date_range, &Timex.beginning_of_month/1, &Timex.end_of_month/1)
   end
 
   defp build_full_intervals(_query, _labels) do
     nil
   end
 
+  def build_intervals(labels, date_range, start_fn, end_fn) do
+    for label <- labels, into: %{} do
+      case Date.from_iso8601(label) do
+        {:ok, date} ->
+          interval_start = start_fn.(date)
+          interval_end = end_fn.(date)
+
+          within_interval? =
+            Enum.member?(date_range, interval_start) && Enum.member?(date_range, interval_end)
+
+          {label, within_interval?}
+
+        _ ->
+          {label, false}
+      end
+    end
+  end
+
   def top_stats(conn, params) do
     site = conn.assigns[:site]
 
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 f04cbacbb..74b7bbaee 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
@@ -683,6 +683,36 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
       assert [0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0] = prev
       assert [0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0] = curr
     end
+
+    test "displays conversions per month with 12mo comparison plot", %{
+      conn: conn,
+      site: site
+    } do
+      insert(:goal, site: site, event_name: "Signup")
+
+      populate_stats(site, [
+        build(:event, name: "Different", timestamp: ~N[2020-01-10 00:00:00]),
+        build(:event, name: "Signup", timestamp: ~N[2020-02-10 00:00:00]),
+        build(:event, name: "Signup", timestamp: ~N[2020-03-10 00:00:00]),
+        build(:event, name: "Signup", timestamp: ~N[2020-04-10 00:00:00]),
+        build(:pageview, timestamp: ~N[2021-05-10 00:00:00]),
+        build(:event, name: "Signup", timestamp: ~N[2021-06-11 04:00:00]),
+        build(:event, name: "Signup", timestamp: ~N[2021-07-11 00:00:00]),
+        build(:event, name: "Signup", timestamp: ~N[2021-08-11 00:00:00])
+      ])
+
+      filters = Jason.encode!(%{goal: "Signup"})
+
+      conn =
+        get(
+          conn,
+          "/api/stats/#{site.domain}/main-graph?period=12mo&date=2021-12-11&metric=events&filters=#{filters}&comparison=previous_period"
+        )
+
+      assert %{"plot" => curr, "comparison_plot" => prev} = json_response(conn, 200)
+      assert [0, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0] = prev
+      assert [0, 0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 0] = curr
+    end
   end
 
   describe "GET /api/stats/main-graph - bounce_rate plot" do