Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ All notable changes to this project will be documented in this file.

- All dropmenus on dashboard are navigable with Tab (used to be a mix between tab and arrow keys), and no two dropmenus can be open at once on the dashboard

- Special path-based events like "404" don't need `event.props.path` to be explicitly defined when tracking: it is set to be the same as `event.pathname` in event ingestion. If it is explicitly defined, it is not overridden for backwards compatibility.

### Fixed

- Make clicking Compare / Disable Comparison in period picker menu close the menu
Expand Down
4 changes: 2 additions & 2 deletions lib/plausible/exports.ex
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ defmodule Plausible.Exports do
fragment(
"if(? in ?, ?, '')",
e.name,
^Plausible.Imported.goals_with_url(),
^Plausible.Goals.SystemGoals.goals_with_url(),
get_by_key(e, :meta, "url")
),
:link_url
Expand All @@ -573,7 +573,7 @@ defmodule Plausible.Exports do
fragment(
"if(? in ?, ?, '')",
e.name,
^Plausible.Imported.goals_with_path(),
^Plausible.Goals.SystemGoals.goals_with_path(),
get_by_key(e, :meta, "path")
),
:path
Expand Down
48 changes: 48 additions & 0 deletions lib/plausible/goals/system_goals.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
defmodule Plausible.Goals.SystemGoals do
@moduledoc """
This module contains logic for special goals
"""

# Special system goals which can be filtered by 'url' custom property
@goals_with_url ["Outbound Link: Click", "Cloaked Link: Click", "File Download"]

# Special system goals which can be filtered by 'path' custom property
@goals_with_path ["404", "WP Form Completions", "Form: Submission"]

@spec goals_with_url() :: [String.t()]
def goals_with_url() do
@goals_with_url
end

@spec goals_with_path() :: [String.t()]
def goals_with_path() do
@goals_with_path
end

@spec special_goals_for(String.t()) :: [String.t()]
def special_goals_for("event:props:url"), do: goals_with_url()
def special_goals_for("event:props:path"), do: goals_with_path()

@doc """
Checks if the event name is for a special goal that should have the event.props.path synced with the event.pathname property.

### Examples
iex> sync_props_path_with_pathname?("404", [{"path", "/foo"}])
false

Note: Should not override event.props.path if it is set deliberately to nil
iex> sync_props_path_with_pathname?("404", [{"path", nil}])
false

iex> sync_props_path_with_pathname?("404", [{"other", "value"}])
true

iex> sync_props_path_with_pathname?("404", [])
true
"""
@spec sync_props_path_with_pathname?(String.t(), [{String.t(), String.t()}]) :: boolean()
def sync_props_path_with_pathname?(event_name, props_in_request) do
event_name in goals_with_path() and
not Enum.any?(props_in_request, fn {k, _} -> k == "path" end)
end
end
15 changes: 0 additions & 15 deletions lib/plausible/imported.ex
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ defmodule Plausible.Imported do
# Maximum number of complete imports to account for when querying stats
@max_complete_imports 5

# Goals which can be filtered by url property
@goals_with_url ["Outbound Link: Click", "Cloaked Link: Click", "File Download"]
# Goals which can be filtered by path property
@goals_with_path ["404", "WP Form Completions"]

@spec schemas() :: [module()]
def schemas, do: @tables

Expand All @@ -56,16 +51,6 @@ defmodule Plausible.Imported do
Enum.map(~w(url path), &("event:props:" <> &1))
end

@spec goals_with_url() :: [String.t()]
def goals_with_url() do
@goals_with_url
end

@spec goals_with_path() :: [String.t()]
def goals_with_path() do
@goals_with_path
end

@spec any_completed_imports?(Site.t()) :: boolean()
def any_completed_imports?(site) do
get_completed_imports(site) != []
Expand Down
15 changes: 14 additions & 1 deletion lib/plausible/ingestion/request.ex
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ defmodule Plausible.Ingestion.Request do
|> put_user_agent(conn)
|> put_request_params(request_body)
|> put_referrer(request_body)
|> put_pathname()
|> put_props(request_body)
|> put_engagement_fields(request_body)
|> put_pathname()
|> put_query_params()
|> put_revenue_source(request_body)
|> put_interactive(request_body)
Expand Down Expand Up @@ -171,6 +171,18 @@ defmodule Plausible.Ingestion.Request do
Changeset.put_change(changeset, :pathname, pathname)
end

defp maybe_set_props_path_to_pathname(props_in_request, changeset) do
if Plausible.Goals.SystemGoals.sync_props_path_with_pathname?(
Changeset.get_field(changeset, :event_name),
props_in_request
) do
# "path" props is added to the head of the props enum to avoid it being cut off
[{"path", Changeset.get_field(changeset, :pathname)}] ++ props_in_request
else
props_in_request
end
end

defp map_domains(changeset, %{} = request_body) do
raw = request_body["d"] || request_body["domain"]
raw = if is_binary(raw), do: String.trim(raw)
Expand Down Expand Up @@ -227,6 +239,7 @@ defmodule Plausible.Ingestion.Request do
(request_body["m"] || request_body["meta"] || request_body["p"] || request_body["props"])
|> Plausible.Helpers.JSON.decode_or_fallback()
|> Enum.reduce([], &filter_bad_props/2)
|> maybe_set_props_path_to_pathname(changeset)
|> Enum.take(@max_props)
|> Map.new()

Expand Down
5 changes: 1 addition & 4 deletions lib/plausible/stats/imported/base.ex
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ defmodule Plausible.Stats.Imported.Base do
[:is, "event:goal", names | _rest] -> names
_ -> []
end)
|> Enum.any?(&(&1 in special_goals_for(property)))
|> Enum.any?(&(&1 in Plausible.Goals.SystemGoals.special_goals_for(property)))

has_unsupported_filters? =
query.filters
Expand Down Expand Up @@ -201,7 +201,4 @@ defmodule Plausible.Stats.Imported.Base do
_ -> []
end
end

def special_goals_for("event:props:url"), do: Imported.goals_with_url()
def special_goals_for("event:props:path"), do: Imported.goals_with_path()
end
12 changes: 2 additions & 10 deletions lib/plausible/stats/imported/imported.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,15 @@ defmodule Plausible.Stats.Imported do

@property_to_table_mappings Imported.Base.property_to_table_mappings()

@goals_with_url Plausible.Imported.goals_with_url()

def goals_with_url(), do: @goals_with_url

@goals_with_path Plausible.Imported.goals_with_path()

def goals_with_path(), do: @goals_with_path

@doc """
Returns a boolean indicating whether the combination of filters and
breakdown property is possible to query from the imported tables.

Usually, when no filters are used, the imported schema supports the
query. There is one exception though - breakdown by a custom property.
We are currently importing only two custom properties - `url` and `path.
We are currently importing only two custom properties - `url` and `path`.
Both these properties can only be used with their special goal filter
(see `@goals_with_url` and `@goals_with_path`).
(see Plausible.Goals.SystemGoals).
"""
def schema_supports_query?(query) do
length(Imported.Base.decide_tables(query)) > 0
Expand Down
2 changes: 2 additions & 0 deletions test/plausible/goals_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ defmodule Plausible.GoalsTest do
use Plausible.Teams.Test
alias Plausible.Goals

doctest Plausible.Goals.SystemGoals, import: true

test "create/2 creates goals and trims input" do
site = new_site()
{:ok, goal} = Goals.create(site, %{"page_path" => "/foo bar "})
Expand Down
49 changes: 49 additions & 0 deletions test/plausible/ingestion/request_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,55 @@ defmodule Plausible.Ingestion.RequestTest do
assert request.pathname == "/pictures/index.html#foo"
end

for event_name <- Plausible.Goals.SystemGoals.goals_with_path() do
test "event.props.path is synced from event.pathname for special path-based event '#{event_name}'" do
payload = %{
name: unquote(event_name),
domain: "dummy.site",
url: "http://dummy.site/pictures/index.html#foo"
}

conn = build_conn(:post, "/api/events", payload)

assert {:ok, request} = Request.build(conn)

assert request.pathname == "/pictures/index.html"
assert request.props == %{"path" => "/pictures/index.html"}
end

test "event.props.path is synced from event.pathname for special path-based event '#{event_name}' with hashMode" do
payload = %{
name: unquote(event_name),
domain: "dummy.site",
url: "http://dummy.site/pictures/index.html#foo",
hashMode: 1
}

conn = build_conn(:post, "/api/events", payload)

assert {:ok, request} = Request.build(conn)

assert request.pathname == "/pictures/index.html#foo"
assert request.props == %{"path" => "/pictures/index.html#foo"}
end

test "event.props.path is not synced from event.pathname for special path-based event '#{event_name}' if it's set explicitly (legacy support)" do
payload = %{
name: unquote(event_name),
domain: "dummy.site",
url: "http://dummy.site/pictures/index.html#foo",
props: %{"path" => "/album"}
}

conn = build_conn(:post, "/api/events", payload)

assert {:ok, request} = Request.build(conn)

assert request.pathname == "/pictures/index.html"
assert request.props == %{"path" => "/album"}
end
end

test "query params are set" do
payload = %{
name: "pageview",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3372,7 +3372,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.BreakdownTest do
assert %{"source" => "Google", "events" => 1} = breakdown_and_first.("visit:source")
end

for goal_name <- Plausible.Imported.goals_with_url() do
for goal_name <- Plausible.Goals.SystemGoals.goals_with_url() do
test "returns url breakdown for #{goal_name} goal", %{conn: conn, site: site} do
insert(:goal, event_name: unquote(goal_name), site: site)
site_import = insert(:site_import, site: site)
Expand Down Expand Up @@ -3432,7 +3432,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.BreakdownTest do
end
end

for goal_name <- Plausible.Imported.goals_with_path() do
for goal_name <- Plausible.Goals.SystemGoals.goals_with_path() do
test "returns path breakdown for #{goal_name} goal", %{conn: conn, site: site} do
insert(:goal, event_name: unquote(goal_name), site: site)
site_import = insert(:site_import, site: site)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryImportedTest do
breakdown_and_first.("visit:source")
end

for goal_name <- Plausible.Imported.goals_with_url() do
for goal_name <- Plausible.Goals.SystemGoals.goals_with_url() do
test "returns url breakdown for #{goal_name} goal", %{conn: conn, site: site} do
insert(:goal, event_name: unquote(goal_name), site: site)
site_import = insert(:site_import, site: site)
Expand Down Expand Up @@ -734,7 +734,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryImportedTest do
end
end

for goal_name <- Plausible.Imported.goals_with_path() do
for goal_name <- Plausible.Goals.SystemGoals.goals_with_path() do
test "returns path breakdown for #{goal_name} goal", %{conn: conn, site: site} do
insert(:goal, event_name: unquote(goal_name), site: site)
site_import = insert(:site_import, site: site)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1261,7 +1261,7 @@ defmodule PlausibleWeb.Api.StatsController.CustomPropBreakdownTest do
]
end

for goal_name <- Plausible.Imported.goals_with_path() do
for goal_name <- Plausible.Goals.SystemGoals.goals_with_path() do
test "returns path breakdown for #{goal_name} goal", %{conn: conn, site: site} do
insert(:goal, event_name: unquote(goal_name), site: site)
site_import = insert(:site_import, site: site)
Expand Down Expand Up @@ -1320,7 +1320,7 @@ defmodule PlausibleWeb.Api.StatsController.CustomPropBreakdownTest do
end
end

for goal_name <- Plausible.Imported.goals_with_url() do
for goal_name <- Plausible.Goals.SystemGoals.goals_with_url() do
test "returns url breakdown for #{goal_name} goal", %{conn: conn, site: site} do
insert(:goal, event_name: unquote(goal_name), site: site)
site_import = insert(:site_import, site: site)
Expand Down
2 changes: 2 additions & 0 deletions tracker/npm_package/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- "Form: Submission" event payload does not need to contain props.path any more: it is saved to be the same as the pathname of the event

## [0.3.1] - 2025-07-08

- Do not send "Form: Submission" event if the form is tagged
Expand Down
2 changes: 1 addition & 1 deletion tracker/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"tracker_script_version": 22,
"tracker_script_version": 23,
"type": "module",
"scripts": {
"deploy": "node compile.js",
Expand Down
2 changes: 1 addition & 1 deletion tracker/src/custom-events.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export function init() {
// If the form is tagged, we don't track it as a generic form submission.
return
}
track('Form: Submission', { props: { path: location.pathname } });
track('Form: Submission');
}
}

Expand Down
Loading
Loading