Fix bug with multiple ARRAY JOIN query (#2653)

* Fix bug with multiple ARRAY JOIN query

* Add changelog entry

* Remove 'bug' label from test description

* Simplify pattern match

Co-authored-by: Adam Rutkowski <hq@mtod.org>

* Rename has_join_with_table -> joins_table?

Co-authored-by: Adam Rutkowski <hq@mtod.org>

---------

Co-authored-by: Adam Rutkowski <hq@mtod.org>

CHANGELOG.md

@@ -15,6 +15,7 @@ All notable changes to this project will be documented in this file.
 - Fix breakdown API pagination when using event metrics plausible/analytics#2562
 - Automatically update all visible dashboard reports in the realtime view
 - Connect via TLS when using HTTPS scheme in ClickHouse URL plausible/analytics#2570
+- Fix bug with [showing property breakdown with a prop filter](https://github.com/plausible/analytics/issues/1789)
 
 ### Changed
 - Reject events with long URIs and data URIs plausible/analytics#2536

lib/plausible/stats/breakdown.ex

@@ -316,20 +316,13 @@ defmodule Plausible.Stats.Breakdown do
     end
   end
 
-  defp do_group_by(
-         %Ecto.Query{
-           from: %Ecto.Query.FromExpr{source: {"events", _}},
-           joins: [%Ecto.Query.JoinExpr{source: {"meta", _}}]
-         } = q,
-         "event:props:" <> prop
-       ) do
-    from(
-      [e, meta] in q,
-      group_by: e.name,
-      where: meta.key == ^prop,
-      group_by: meta.value,
-      select_merge: %{^prop => meta.value},
-      order_by: {:asc, meta.value}
+  defp joins_table?(ecto_q, table) do
+    Enum.any?(
+      ecto_q.joins,
+      fn
+        %Ecto.Query.JoinExpr{source: {^table, _}} -> true
+        _ -> false
+      end
     )
   end
 
@@ -337,9 +330,18 @@ defmodule Plausible.Stats.Breakdown do
          %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"events", _}}} = q,
          "event:props:" <> prop
        ) do
+    q =
+      if joins_table?(q, "meta") do
+        q
+      else
+        from(
+          e in q,
+          inner_lateral_join: meta in fragment("meta")
+        )
+      end
+
     from(
-      e in q,
-      inner_lateral_join: meta in fragment("meta"),
+      [e, meta] in q,
       where: meta.key == ^prop,
       group_by: meta.value,
       select_merge: %{^prop => meta.value},

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

@@ -352,6 +352,51 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
                }
              ]
     end
+
+    test "Property breakdown with prop and goal filter", %{conn: conn, site: site} do
+      populate_stats(site, [
+        build(:pageview, user_id: 1, utm_campaign: "campaignA"),
+        build(:event,
+          user_id: 1,
+          name: "ButtonClick",
+          "meta.key": ["variant"],
+          "meta.value": ["A"]
+        ),
+        build(:pageview, user_id: 2, utm_campaign: "campaignA"),
+        build(:event,
+          user_id: 2,
+          name: "ButtonClick",
+          "meta.key": ["variant"],
+          "meta.value": ["B"]
+        )
+      ])
+
+      insert(:goal, %{domain: site.domain, event_name: "ButtonClick"})
+
+      filters =
+        Jason.encode!(%{
+          goal: "ButtonClick",
+          props: %{variant: "A"},
+          utm_campaign: "campaignA"
+        })
+
+      prop_key = "variant"
+
+      conn =
+        get(
+          conn,
+          "/api/stats/#{site.domain}/property/#{prop_key}?period=day&filters=#{filters}"
+        )
+
+      assert json_response(conn, 200) == [
+               %{
+                 "name" => "A",
+                 "unique_conversions" => 1,
+                 "total_conversions" => 1,
+                 "conversion_rate" => 50.0
+               }
+             ]
+    end
   end
 
   describe "GET /api/stats/:domain/conversions - with glob goals" do