fix(revenue): Fix selecting and graphing revenue goals with compariso…

…ns (plausible#4680)

* Support graphing revenue goals in dashboard again

This has been broken since the introduction of display names

* Support graphing revenue metrics timeseries with comparisons

This was previously broken with an ArithmeticError and didn't have any coverage.

Related sentry report: https://sentry.plausible.io/organizations/plausible/issues/560/?project=2&query=is%3Aunresolved+issue.priority%3A%5Bhigh%2C+medium%5D&referrer=issue-stream&statsPeriod=14d&stream_index=1

* Simplify

* NPM test update

* Update goals test

assets/js/dashboard/query.ts

@@ -158,7 +158,7 @@ export function revenueAvailable(query: DashboardQuery, site: PlausibleSite) {
     const goalFilters: Filter[] = getFiltersByKeyPrefix(query, 'goal')
 
     return goalFilters.some(([_op, _key, clauses]) => {
-      return clauses.includes(rg.event_name)
+      return clauses.includes(rg.display_name)
     })
   })
 

assets/js/dashboard/site-context.test.tsx

@@ -17,7 +17,7 @@ describe('parseSiteFromDataset', () => {
       data-props-opted-out="false"
       data-funnels-available="true"
       data-props-available="true"
-      data-revenue-goals='[{"currency":"USD","event_name":"Purchase"}]'
+      data-revenue-goals='[{"currency":"USD","display_name":"Purchase"}]'
       data-funnels='[{"id":1,"name":"From homepage to login","steps_count":3}]'
       data-has-props="true"
       data-logged-in="true"
@@ -40,7 +40,7 @@ describe('parseSiteFromDataset', () => {
     propsOptedOut: false,
     funnelsAvailable: true,
     propsAvailable: true,
-    revenueGoals: [{ currency: 'USD', event_name: 'Purchase' }],
+    revenueGoals: [{ currency: 'USD', display_name: 'Purchase' }],
     funnels: [{ id: 1, name: 'From homepage to login', steps_count: 3 }],
     hasProps: true,
     statsBegin: '2021-09-07',

assets/js/dashboard/site-context.tsx

@@ -36,7 +36,7 @@ const siteContextDefaultValue = {
   conversionsOptedOut: false,
   funnelsOptedOut: false,
   propsOptedOut: false,
-  revenueGoals: [] as { event_name: string; currency: 'USD' }[],
+  revenueGoals: [] as { display_name: string; currency: 'USD' }[],
   funnels: [] as { id: number; name: string; steps_count: number }[],
   /** date in YYYY-MM-DD, @example "2023-01-01" */
   statsBegin: '',

lib/plausible/goals/goals.ex

@@ -111,7 +111,7 @@ defmodule Plausible.Goals do
   def list_revenue_goals(site) do
     from(g in Plausible.Goal,
       where: g.site_id == ^site.id and not is_nil(g.currency),
-      select: %{event_name: g.event_name, currency: g.currency}
+      select: %{display_name: g.display_name, currency: g.currency}
     )
     |> Plausible.Repo.all()
   end

lib/plausible/stats/query_runner.ex

@@ -254,7 +254,18 @@ defmodule Plausible.Stats.QueryRunner do
     Map.get_lazy(comparison_map, dimensions, fn -> empty_metrics(query) end)
   end
 
-  defp empty_metrics(query), do: List.duplicate(0, length(query.metrics))
+  defp empty_metrics(query) do
+    query.metrics
+    |> Enum.map(fn metric -> empty_metric_value(metric) end)
+  end
+
+  on_ee do
+    defp empty_metric_value(metric)
+         when metric in [:total_revenue, :average_revenue],
+         do: nil
+  end
+
+  defp empty_metric_value(_), do: 0
 
   defp total_rows([]), do: 0
   defp total_rows([first_row | _rest]), do: first_row.total_rows

test/plausible/goals_test.exs

@@ -129,9 +129,9 @@ defmodule Plausible.GoalsTest do
     revenue_goals = Goals.list_revenue_goals(site)
 
     assert length(revenue_goals) == 3
-    assert %{event_name: "One", currency: :EUR} in revenue_goals
-    assert %{event_name: "Two", currency: :EUR} in revenue_goals
-    assert %{event_name: "Three", currency: :USD} in revenue_goals
+    assert %{display_name: "One", currency: :EUR} in revenue_goals
+    assert %{display_name: "Two", currency: :EUR} in revenue_goals
+    assert %{display_name: "Three", currency: :USD} in revenue_goals
   end
 
   test "create/2 clears currency for pageview goals" do

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

@@ -1368,6 +1368,61 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
                30.31
              ]
     end
+
+    test "plots total_revenue for a week compared to last week", %{conn: conn, site: site} do
+      insert(:goal,
+        site: site,
+        event_name: "Payment",
+        currency: "USD",
+        display_name: "PaymentUSD"
+      )
+
+      populate_stats(site, [
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("13.29"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-01 00:00:00]
+        ),
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("19.90"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-05 00:00:00]
+        ),
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("10.31"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-10 00:00:00]
+        ),
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("20.0"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-12 00:00:00]
+        ),
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("10.0"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-12 01:00:00]
+        )
+      ])
+
+      filters = Jason.encode!(%{goal: "PaymentUSD"})
+
+      conn =
+        get(
+          conn,
+          "/api/stats/#{site.domain}/main-graph?period=7d&date=2021-01-14&metric=total_revenue&filters=#{filters}&comparison=previous_period"
+        )
+
+      assert %{"plot" => plot, "comparison_plot" => prev} = json_response(conn, 200)
+
+      assert plot == [0.0, 0.0, 10.31, 0.0, 30.0, 0.0, 0.0]
+      assert prev == [13.29, 0.0, 0.0, 0.0, 19.9, 0.0, 0.0]
+    end
   end
 
   describe "GET /api/stats/main-graph - average_revenue plot" do
@@ -1454,6 +1509,61 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
                15.155
              ]
     end
+
+    test "plots average_revenue for a week compared to last week", %{conn: conn, site: site} do
+      insert(:goal,
+        site: site,
+        event_name: "Payment",
+        currency: "USD",
+        display_name: "PaymentUSD"
+      )
+
+      populate_stats(site, [
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("13.29"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-01 00:00:00]
+        ),
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("19.90"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-05 00:00:00]
+        ),
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("10.31"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-10 00:00:00]
+        ),
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("20.0"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-12 00:00:00]
+        ),
+        build(:event,
+          name: "Payment",
+          revenue_reporting_amount: Decimal.new("10.0"),
+          revenue_reporting_currency: "USD",
+          timestamp: ~N[2021-01-12 01:00:00]
+        )
+      ])
+
+      filters = Jason.encode!(%{goal: "PaymentUSD"})
+
+      conn =
+        get(
+          conn,
+          "/api/stats/#{site.domain}/main-graph?period=7d&date=2021-01-14&metric=average_revenue&filters=#{filters}&comparison=previous_period"
+        )
+
+      assert %{"plot" => plot, "comparison_plot" => prev} = json_response(conn, 200)
+
+      assert plot == [0.0, 0.0, 10.31, 0.0, 15.0, 0.0, 0.0]
+      assert prev == [13.29, 0.0, 0.0, 0.0, 19.9, 0.0, 0.0]
+    end
   end
 
   describe "present_index" do