Skip to content

Commit

Permalink
Average Scroll Depth Metric: extracted ingestion changes (#4828)
Browse files Browse the repository at this point in the history
* migration: add scroll_depth to events_v2

* (cherry-pick) ingest scroll depth

* replace convoluted test with more concise ones
  • Loading branch information
RobertJoonas authored Nov 19, 2024
1 parent e93c97d commit 7f78392
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 2 deletions.
2 changes: 2 additions & 0 deletions lib/plausible/clickhouse_event_v2.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ defmodule Plausible.ClickhouseEventV2 do

field :"meta.key", {:array, :string}
field :"meta.value", {:array, :string}
field :scroll_depth, Ch, type: "UInt8"

field :revenue_source_amount, Ch, type: "Nullable(Decimal64(3))"
field :revenue_source_currency, Ch, type: "FixedString(3)"
Expand Down Expand Up @@ -60,6 +61,7 @@ defmodule Plausible.ClickhouseEventV2 do
:timestamp,
:"meta.key",
:"meta.value",
:scroll_depth,
:revenue_source_amount,
:revenue_source_currency,
:revenue_reporting_amount,
Expand Down
3 changes: 2 additions & 1 deletion lib/plausible/ingestion/event.ex
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,8 @@ defmodule Plausible.Ingestion.Event do
timestamp: event.request.timestamp,
name: event.request.event_name,
hostname: event.request.hostname,
pathname: event.request.pathname
pathname: event.request.pathname,
scroll_depth: event.request.scroll_depth
})
end

Expand Down
17 changes: 17 additions & 0 deletions lib/plausible/ingestion/request.ex
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ defmodule Plausible.Ingestion.Request do
field :hash_mode, :integer
field :pathname, :string
field :props, :map
field :scroll_depth, :integer

on_ee do
field :revenue_source, :map
Expand Down Expand Up @@ -77,6 +78,7 @@ defmodule Plausible.Ingestion.Request do
|> put_request_params(request_body)
|> put_referrer(request_body)
|> put_props(request_body)
|> put_scroll_depth(request_body)
|> put_pathname()
|> put_query_params()
|> put_revenue_source(request_body)
Expand Down Expand Up @@ -245,6 +247,21 @@ defmodule Plausible.Ingestion.Request do
end
end

defp put_scroll_depth(changeset, %{} = request_body) do
if Changeset.get_field(changeset, :event_name) == "pageleave" do
scroll_depth =
case request_body["sd"] do
sd when is_integer(sd) and sd >= 0 and sd <= 100 -> sd
sd when is_integer(sd) and sd > 100 -> 100
_ -> 0
end

Changeset.put_change(changeset, :scroll_depth, scroll_depth)
else
changeset
end
end

defp put_query_params(changeset) do
case Changeset.get_field(changeset, :uri) do
%{query: query} when is_binary(query) ->
Expand Down
3 changes: 2 additions & 1 deletion test/plausible/ingestion/request_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,8 @@ defmodule Plausible.Ingestion.RequestTest do
"revenue_source" => %{"amount" => "12.3", "currency" => "USD"},
"uri" => "https://dummy.site/pictures/index.html?foo=bar&baz=bam",
"user_agent" => "Mozilla",
"ip_classification" => nil
"ip_classification" => nil,
"scroll_depth" => nil
}

assert %NaiveDateTime{} = NaiveDateTime.from_iso8601!(request["timestamp"])
Expand Down
58 changes: 58 additions & 0 deletions test/plausible_web/controllers/api/external_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1253,6 +1253,64 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
end
end

describe "scroll depth tests" do
setup do
site = insert(:site)
{:ok, site: site}
end

test "ingests scroll_depth as 0 when sd not in params", %{conn: conn, site: site} do
post(conn, "/api/event", %{n: "pageview", u: "https://test.com", d: site.domain})
post(conn, "/api/event", %{n: "pageleave", u: "https://test.com", d: site.domain})
post(conn, "/api/event", %{n: "custom", u: "https://test.com", d: site.domain})

assert [%{scroll_depth: 0}, %{scroll_depth: 0}, %{scroll_depth: 0}] = get_events(site)
end

test "sd field is ignored if name is not pageleave", %{conn: conn, site: site} do
post(conn, "/api/event", %{n: "pageview", u: "https://test.com", d: site.domain, sd: 10})
post(conn, "/api/event", %{n: "custom_e", u: "https://test.com", d: site.domain, sd: 10})

assert [%{scroll_depth: 0}, %{scroll_depth: 0}] = get_events(site)
end

test "ingests valid scroll_depth for a pageleave", %{conn: conn, site: site} do
post(conn, "/api/event", %{n: "pageview", u: "https://test.com", d: site.domain})
post(conn, "/api/event", %{n: "pageleave", u: "https://test.com", d: site.domain, sd: 25})

pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))

assert pageleave.scroll_depth == 25
end

test "ingests scroll_depth as 100 when sd > 100", %{conn: conn, site: site} do
post(conn, "/api/event", %{n: "pageview", u: "https://test.com", d: site.domain})
post(conn, "/api/event", %{n: "pageleave", u: "https://test.com", d: site.domain, sd: 101})

pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))

assert pageleave.scroll_depth == 100
end

test "ingests scroll_depth as 0 when sd is a string", %{conn: conn, site: site} do
post(conn, "/api/event", %{n: "pageview", u: "https://test.com", d: site.domain})
post(conn, "/api/event", %{n: "pageleave", u: "https://test.com", d: site.domain, sd: "1"})

pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))

assert pageleave.scroll_depth == 0
end

test "ingests scroll_depth as 0 when sd is a negative integer", %{conn: conn, site: site} do
post(conn, "/api/event", %{n: "pageview", u: "https://test.com", d: site.domain})
post(conn, "/api/event", %{n: "pageleave", u: "https://test.com", d: site.domain, sd: -1})

pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))

assert pageleave.scroll_depth == 0
end
end

describe "acquisition channel tests" do
setup do
site = insert(:site)
Expand Down

0 comments on commit 7f78392

Please sign in to comment.