Automatically refresh query struct state after modifications

lib/plausible/imported.ex

@@ -54,6 +54,11 @@ defmodule Plausible.Imported do
     @max_complete_imports
   end
 
+  @spec imported_custom_props() :: [String.t()]
+  def imported_custom_props do
+    Plausible.Props.internal_keys()
+  end
+
   @spec goals_with_url() :: [String.t()]
   def goals_with_url() do
     @goals_with_url

lib/plausible/stats/base.ex

@@ -308,7 +308,10 @@ defmodule Plausible.Stats.Base do
 
   def add_percentage_metric(q, site, query, metrics) do
     if :percentage in metrics do
-      query = struct!(query, property: nil)
+      query =
+        query
+        |> Query.set_property(nil)
+        |> Query.exclude_imported()
 
       q
       |> select_merge(^%{__total_visitors: total_visitors_subquery(site, query)})
@@ -334,7 +337,20 @@ defmodule Plausible.Stats.Base do
       total_query =
         query
         |> Query.remove_filters(["event:goal", "event:props"])
-        |> struct!(property: nil)
+        |> Query.set_property(nil)
+
+      has_custom_prop_filters? =
+        Enum.any?(query.filters, fn
+          [_, "event:props:" <> prop, _] -> prop not in Plausible.Imported.imported_custom_props()
+          _ -> false
+        end)
+
+      total_query =
+        if has_custom_prop_filters? do
+          Query.exclude_imported(total_query)
+        else
+          total_query
+        end
 
       # :TRICKY: Subquery is used due to event:goal breakdown above doing an UNION ALL
       subquery(q)

lib/plausible/stats/breakdown.ex

@@ -41,7 +41,6 @@ defmodule Plausible.Stats.Breakdown do
       query
       |> Query.put_filter([:member, "event:name", events])
       |> Query.set_property("event:name")
-      |> Query.refresh(site)
 
     if !Keyword.get(opts, :skip_tracing), do: Query.trace(query, metrics)
 
@@ -74,11 +73,7 @@ defmodule Plausible.Stats.Breakdown do
 
     page_q =
       if Enum.any?(pageview_goals) do
-        page_query = struct!(query, property: "event:page")
-
-        query
-        |> Query.set_property("event:page")
-        |> Query.refresh(site)
+        page_query = Query.set_property(query, "event:page")
 
         page_exprs = Enum.map(pageview_goals, & &1.page_path)
         page_regexes = Enum.map(page_exprs, &page_regex/1)
@@ -189,7 +184,6 @@ defmodule Plausible.Stats.Breakdown do
           |> Query.remove_filters(["event:page"])
           |> Query.put_filter([:member, "visit:entry_page", Enum.map(pages, & &1[:page])])
           |> Query.set_property("visit:entry_page")
-          |> Query.refresh(site)
       end
 
     if Enum.any?(event_metrics) && Enum.empty?(event_result) do
@@ -269,7 +263,8 @@ defmodule Plausible.Stats.Breakdown do
         query
 
       [op, "event:hostname", value] ->
-        Plausible.Stats.Query.put_filter(query, [op, visit_prop, value])
+        query
+        |> Query.put_filter([op, visit_prop, value])
     end
   end
 

lib/plausible/stats/comparisons.ex

@@ -63,7 +63,7 @@ defmodule Plausible.Stats.Comparisons do
 
     with :ok <- validate_mode(source_query, mode),
          {:ok, comparison_query} <- do_compare(source_query, mode, opts),
-         comparison_query <- maybe_include_imported(comparison_query, source_query, site),
+         comparison_query <- maybe_include_imported(comparison_query, source_query),
          do: {:ok, comparison_query}
   end
 
@@ -162,10 +162,10 @@ defmodule Plausible.Stats.Comparisons do
     Date.add(date, -days_to_subtract)
   end
 
-  defp maybe_include_imported(query, source_query, site) do
+  defp maybe_include_imported(query, source_query) do
     requested? = source_query.imported_data_requested
 
-    case Query.ensure_include_imported(query, site, requested?) do
+    case Query.ensure_include_imported(query, requested?) do
       :ok ->
         struct!(query,
           imported_data_requested: true,

lib/plausible/stats/email_report.ex

@@ -40,7 +40,7 @@ defmodule Plausible.Stats.EmailReport do
   end
 
   defp put_top_5_pages(stats, site, query) do
-    query = struct!(query, property: "event:page")
+    query = Query.set_property(query, "event:page")
     pages = Stats.breakdown(site, query, [:visitors], {5, 1})
     Map.put(stats, :pages, pages)
   end
@@ -49,7 +49,7 @@ defmodule Plausible.Stats.EmailReport do
     query =
       query
       |> Query.put_filter([:is_not, "visit:source", "Direct / None"])
-      |> struct!(property: "visit:source")
+      |> Query.set_property("visit:source")
 
     sources = Stats.breakdown(site, query, [:visitors], {5, 1})
 

lib/plausible/stats/query.ex

@@ -12,7 +12,8 @@ defmodule Plausible.Stats.Query do
             skip_imported_reason: nil,
             now: nil,
             experimental_session_count?: false,
-            experimental_reduced_joins?: false
+            experimental_reduced_joins?: false,
+            latest_import_end_date: nil
 
   require OpenTelemetry.Tracer, as: Tracer
   alias Plausible.Stats.{Filters, Interval, Imported}
@@ -204,18 +205,15 @@ defmodule Plausible.Stats.Query do
 
   @spec set_property(t(), String.t()) :: t()
   def set_property(query, property) do
-    struct!(query, property: property)
+    query
+    |> struct!(property: property)
+    |> refresh()
   end
 
   def put_filter(query, filter) do
-    struct!(query,
-      filters: query.filters ++ [filter]
-    )
-  end
-
-  @spec refresh(t(), Plausible.Site.t()) :: t()
-  def refresh(query, site) do
-    put_imported_opts(query, site)
+    query
+    |> struct!(filters: query.filters ++ [filter])
+    |> refresh()
   end
 
   def remove_filters(query, prefixes) do
@@ -224,7 +222,20 @@ defmodule Plausible.Stats.Query do
         Enum.any?(prefixes, &String.starts_with?(filter_key, &1))
       end)
 
-    struct!(query, filters: new_filters)
+    query
+    |> struct!(filters: new_filters)
+    |> refresh()
+  end
+
+  def exclude_imported(query) do
+    struct!(query,
+      include_imported: false,
+      skip_imported_reason: :manual_exclusion
+    )
+  end
+
+  defp refresh(query) do
+    refresh_imported_opts(query)
   end
 
   def has_event_filters?(query) do
@@ -257,10 +268,23 @@ defmodule Plausible.Stats.Query do
     end
   end
 
-  defp put_imported_opts(query, site, params \\ %{}) do
+  defp refresh_imported_opts(query) do
+    put_imported_opts(query, nil, %{})
+  end
+
+  defp put_imported_opts(query, site, params) do
     requested? = params["with_imported"] == "true" || query.imported_data_requested
 
-    case ensure_include_imported(query, site, requested?) do
+    latest_import_end_date =
+      if site do
+        site.latest_import_end_date
+      else
+        query.latest_import_end_date
+      end
+
+    query = struct!(query, latest_import_end_date: latest_import_end_date)
+
+    case ensure_include_imported(query, requested?) do
       :ok ->
         struct!(query,
           imported_data_requested: true,
@@ -276,13 +300,13 @@ defmodule Plausible.Stats.Query do
     end
   end
 
-  @spec ensure_include_imported(t(), Plausible.Site.t(), boolean()) ::
+  @spec ensure_include_imported(t(), boolean()) ::
           :ok | {:error, :not_requested | :no_imported_data | :out_of_range | :unsupported_query}
-  def ensure_include_imported(query, site, requested?) do
+  def ensure_include_imported(query, requested?) do
     cond do
       not requested? -> {:error, :not_requested}
-      is_nil(site.latest_import_end_date) -> {:error, :no_imported_data}
-      Date.after?(query.date_range.first, site.latest_import_end_date) -> {:error, :out_of_range}
+      is_nil(query.latest_import_end_date) -> {:error, :no_imported_data}
+      Date.after?(query.date_range.first, query.latest_import_end_date) -> {:error, :out_of_range}
       not Imported.schema_supports_query?(query) -> {:error, :unsupported_query}
       query.period == "realtime" -> {:error, :unsupported_query}
       true -> :ok

lib/plausible_web/controllers/api/external_stats_controller.ex

@@ -380,7 +380,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do
   end
 
   defp maybe_add_warning(payload, %{skip_imported_reason: reason})
-       when reason in [nil, :not_requested, :no_imported_data, :out_of_range] do
+       when reason in [nil, :not_requested, :no_imported_data, :out_of_range, :manual_exclusion] do
     payload
   end
 

lib/plausible_web/controllers/api/stats_controller.ex

@@ -872,7 +872,7 @@ defmodule PlausibleWeb.Api.StatsController do
         |> Query.remove_filters(["visit:exit_page"])
         |> Query.put_filter([:member, "event:page", pages])
         |> Query.put_filter([:is, "event:name", "pageview"])
-        |> struct!(property: "event:page")
+        |> Query.set_property("event:page")
 
       total_pageviews =
         Stats.breakdown(site, total_pageviews_query, [:pageviews], {limit, 1})

test/plausible_web/controllers/api/external_stats_controller/aggregate_test.exs

@@ -681,6 +681,34 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AggregateTest do
 
       refute json_response(conn, 200)["warning"]
     end
+
+    test "excludes imported data from conversion rate computation when query filters by non-imported props",
+         %{conn: conn, site: site, site_import: site_import} do
+      insert(:goal, site: site, event_name: "Purchase")
+
+      populate_stats(site, site_import.id, [
+        build(:event,
+          name: "Purchase",
+          "meta.key": ["package"],
+          "meta.value": ["large"]
+        ),
+        build(:imported_visitors, visitors: 9)
+      ])
+
+      conn =
+        get(conn, "/api/v1/stats/aggregate", %{
+          "site_id" => site.domain,
+          "period" => "day",
+          "metrics" => "visitors,conversion_rate",
+          "filters" => "event:goal==Purchase;event:props:package==large",
+          "with_imported" => "true"
+        })
+
+      assert json_response(conn, 200)["results"] == %{
+               "visitors" => %{"value" => 1},
+               "conversion_rate" => %{"value" => 100.0}
+             }
+    end
   end
 
   describe "filters" do

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

@@ -65,6 +65,8 @@ defmodule PlausibleWeb.Api.StatsController.CustomPropBreakdownTest do
                  "percentage" => 100.0
                }
              ]
+
+      refute json_response(conn, 200)["warning"]
     end
 
     test "returns (none) values in the breakdown", %{conn: conn, site: site} do