diff --git a/CHANGELOG.md b/CHANGELOG.md index 575abdb389a4..0d5de65c00de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ All notable changes to this project will be documented in this file. - 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) +- Fix bug when combining goal and prop filters plausible/analytics#2654 ### Changed - Reject events with long URIs and data URIs plausible/analytics#2536 diff --git a/lib/plausible/stats/props.ex b/lib/plausible/stats/props.ex index 7f0b2d1c4fc6..5ae3bb879da2 100644 --- a/lib/plausible/stats/props.ex +++ b/lib/plausible/stats/props.ex @@ -6,13 +6,12 @@ defmodule Plausible.Stats.Props do prop_filter = Enum.find(query.filters, fn {key, _} -> String.starts_with?(key, "event:props:") end) - if prop_filter do - {key, val} = prop_filter + case prop_filter do + {"event:props:" <> key, {_, "(none)"}} -> + {_, _, goal_name} = query.filters["event:goal"] + %{goal_name => [key]} - if val == "(none)" do - goal = query.filters["goal"] - %{goal => [key]} - else + {"event:props:" <> _key, _} -> ClickhouseRepo.all( from [e, meta: meta] in base_event_query(site, query), select: {e.name, meta.key}, @@ -21,17 +20,17 @@ defmodule Plausible.Stats.Props do |> Enum.reduce(%{}, fn {goal_name, meta_key}, acc -> Map.update(acc, goal_name, [meta_key], fn list -> [meta_key | list] end) end) - end - else - ClickhouseRepo.all( - from e in base_event_query(site, query), - inner_lateral_join: meta in fragment("meta as m"), - select: {e.name, meta.key}, - distinct: true - ) - |> Enum.reduce(%{}, fn {goal_name, meta_key}, acc -> - Map.update(acc, goal_name, [meta_key], fn list -> [meta_key | list] end) - end) + + nil -> + ClickhouseRepo.all( + from e in base_event_query(site, query), + inner_lateral_join: meta in fragment("meta as m"), + select: {e.name, meta.key}, + distinct: true + ) + |> Enum.reduce(%{}, fn {goal_name, meta_key}, acc -> + Map.update(acc, goal_name, [meta_key], fn list -> [meta_key | list] end) + end) end end end diff --git a/test/plausible_web/controllers/api/stats_controller/conversions_test.exs b/test/plausible_web/controllers/api/stats_controller/conversions_test.exs index 83930e53e15e..eb468b7ca0fd 100644 --- a/test/plausible_web/controllers/api/stats_controller/conversions_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/conversions_test.exs @@ -250,6 +250,39 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do end end + describe "GET /api/stats/:domain/conversions - with goal and prop=(none) filter" do + setup [:create_user, :log_in, :create_new_site] + + test "returns only the conversion that is filtered for", %{conn: conn, site: site} do + populate_stats(site, [ + build(:pageview, pathname: "/", user_id: 1), + build(:pageview, pathname: "/", user_id: 2), + build(:event, name: "Signup", user_id: 1, "meta.key": ["variant"], "meta.value": ["A"]), + build(:event, name: "Signup", user_id: 2) + ]) + + insert(:goal, %{domain: site.domain, event_name: "Signup"}) + + filters = Jason.encode!(%{goal: "Signup", props: %{variant: "(none)"}}) + + conn = + get( + conn, + "/api/stats/#{site.domain}/conversions?period=day&filters=#{filters}" + ) + + assert json_response(conn, 200) == [ + %{ + "name" => "Signup", + "unique_conversions" => 1, + "total_conversions" => 1, + "prop_names" => ["variant"], + "conversion_rate" => 50 + } + ] + end + end + describe "GET /api/stats/:domain/property/:key" do setup [:create_user, :log_in, :create_new_site]