Skip to content

Commit

Permalink
Handle missing scroll depth #2 (#5017)
Browse files Browse the repository at this point in the history
* add migration

* move scroll_depth_enabled? fn

* maybe set engagement_metrics_enabled_at when requesting dashboard

* maybe set engagement_metrics_enabled_at in shared_link action

* maybe set engagement_metrics_enabled_at on full export

* fix tests

* feature gate scroll depth on the dashboard with site.engagement_metrics_enabled_at

* feature gate scroll depth in full export too

* fix npm ci

* Rename things into FE, remove unneccessary flag checks

* Continue with renaming

* Rename site flag to be more descriptive

* Move business logic, calculate based on scroll depth, make more precalculatable

* Some docs

* Rename to site.scrollDepthVisible in frontend

* Update migration

* Fix template

* Remove boilerplate from tests

* Update tests

* More straight-forward test

* Update condition

---------

Co-authored-by: Robert Joonas <[email protected]>
  • Loading branch information
macobo and RobertJoonas authored Jan 29, 2025
1 parent 94799b6 commit 7c6ba04
Show file tree
Hide file tree
Showing 19 changed files with 455 additions and 164 deletions.
2 changes: 2 additions & 0 deletions assets/js/dashboard/site-context.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('parseSiteFromDataset', () => {
data-revenue-goals='[{"currency":"USD","display_name":"Purchase"}]'
data-funnels='[{"id":1,"name":"From homepage to login","steps_count":3}]'
data-has-props="true"
data-scroll-depth-visible="true"
data-logged-in="true"
data-stats-begin="2021-09-07"
data-native-stats-begin="2022-09-02"
Expand All @@ -43,6 +44,7 @@ describe('parseSiteFromDataset', () => {
revenueGoals: [{ currency: 'USD', display_name: 'Purchase' }],
funnels: [{ id: 1, name: 'From homepage to login', steps_count: 3 }],
hasProps: true,
scrollDepthVisible: true,
statsBegin: '2021-09-07',
nativeStatsBegin: '2022-09-02',
embedded: false,
Expand Down
2 changes: 2 additions & 0 deletions assets/js/dashboard/site-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export function parseSiteFromDataset(dataset: DOMStringMap): PlausibleSite {
offset: parseInt(dataset.offset!, 10),
hasGoals: dataset.hasGoals === 'true',
hasProps: dataset.hasProps === 'true',
scrollDepthVisible: dataset.scrollDepthVisible === 'true',
funnelsAvailable: dataset.funnelsAvailable === 'true',
propsAvailable: dataset.propsAvailable === 'true',
conversionsOptedOut: dataset.conversionsOptedOut === 'true',
Expand Down Expand Up @@ -36,6 +37,7 @@ const siteContextDefaultValue = {
offset: 0,
hasGoals: false,
hasProps: false,
scrollDepthVisible: false,
funnelsAvailable: false,
propsAvailable: false,
conversionsOptedOut: false,
Expand Down
2 changes: 1 addition & 1 deletion assets/js/dashboard/stats/graph/graph-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function getGraphableMetrics(query, site) {
return ["visitors", "events", "conversion_rate"]
} else if (isPageFilter) {
const pageFilterMetrics = ["visitors", "visits", "pageviews", "bounce_rate"]
return site.flags.scroll_depth ? [...pageFilterMetrics, "scroll_depth"] : pageFilterMetrics
return site.scrollDepthVisible ? [...pageFilterMetrics, "scroll_depth"] : pageFilterMetrics
} else {
return ["visitors", "visits", "pageviews", "views_per_visit", "bounce_rate", "visit_duration"]
}
Expand Down
2 changes: 1 addition & 1 deletion assets/js/dashboard/stats/modals/pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function PagesModal() {
metrics.createTimeOnPage()
]

return site.flags.scroll_depth ? [...defaultMetrics, metrics.createScrollDepth()] : defaultMetrics
return site.scrollDepthVisible ? [...defaultMetrics, metrics.createScrollDepth()] : defaultMetrics
}

return (
Expand Down
1 change: 1 addition & 0 deletions assets/test-utils/app-context-providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const TestContextProviders = ({
offset: 0,
hasGoals: false,
hasProps: false,
scrollDepthVisible: false,
funnelsAvailable: false,
propsAvailable: false,
conversionsOptedOut: false,
Expand Down
5 changes: 2 additions & 3 deletions lib/plausible/exports.ex
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,7 @@ defmodule Plausible.Exports do
site = Plausible.Repo.get(Plausible.Site, site_id)
current_user = current_user_id && Plausible.Repo.get(Plausible.Auth.User, current_user_id)

scroll_depth_enabled? =
PlausibleWeb.Api.StatsController.scroll_depth_enabled?(site, current_user)
include_scroll_depth? = Plausible.Stats.ScrollDepth.check_feature_visible!(site, current_user)

base_q =
from(e in sampled("events_v2"),
Expand All @@ -429,7 +428,7 @@ defmodule Plausible.Exports do
order_by: selected_as(:date)
)

if scroll_depth_enabled? do
if include_scroll_depth? do
max_scroll_depth_per_visitor_q =
from(e in "events_v2",
where: ^export_filter(site_id, date_range),
Expand Down
1 change: 1 addition & 0 deletions lib/plausible/site.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ defmodule Plausible.Site do
field :conversions_enabled, :boolean, default: true
field :props_enabled, :boolean, default: true
field :funnels_enabled, :boolean, default: true
field :scroll_depth_visible_at, :naive_datetime

field :ingest_rate_limit_scale_seconds, :integer, default: 60
# default is set via changeset/2
Expand Down
8 changes: 8 additions & 0 deletions lib/plausible/sites.ex
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,14 @@ defmodule Plausible.Sites do
|> Repo.update!()
end

def set_scroll_depth_visible_at(site) do
utc_now = NaiveDateTime.utc_now(:second)

site
|> Ecto.Changeset.change(%{scroll_depth_visible_at: utc_now})
|> Repo.update()
end

def has_goals?(site) do
Repo.exists?(
from(g in Plausible.Goal,
Expand Down
63 changes: 63 additions & 0 deletions lib/plausible/stats/scroll_depth.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
defmodule Plausible.Stats.ScrollDepth do
@moduledoc """
Module to check whether the scroll depth metric is available and visible for a site.
"""

import Ecto.Query
require Logger

alias Plausible.ClickhouseRepo

def feature_available?(site, user) do
FunWithFlags.enabled?(:scroll_depth, for: user) ||
FunWithFlags.enabled?(:scroll_depth, for: site)
end

def feature_visible?(site, user) do
feature_available?(site, user) && not is_nil(site.scroll_depth_visible_at)
end

@doc """
Checks whether the scroll depth feature is visible for a site and updates the site record if it is.
Note this function queries ClickHouse and may take a while to complete.
"""
def check_feature_visible!(site, user) do
cond do
not feature_available?(site, user) ->
false

not is_nil(site.scroll_depth_visible_at) ->
true

is_nil(site.scroll_depth_visible_at) ->
visible? = has_scroll_data_last_30d?(site)

if visible? do
Plausible.Sites.set_scroll_depth_visible_at(site)
end

visible?
end
end

defp has_scroll_data_last_30d?(site) do
try do
ClickhouseRepo.exists?(
from(e in "events_v2",
where:
e.site_id == ^site.id and
e.name == "pageleave" and
e.timestamp >= fragment("toStartOfDay(now() - toIntervalDay(30))") and
e.scroll_depth > 0 and e.scroll_depth <= 100
)
)
rescue
# Avoid propagating error to the user, bringing down the site.
error ->
Logger.error("Error checking scroll data for site #{site.id}: #{inspect(error)}")

false
end
end
end
13 changes: 4 additions & 9 deletions lib/plausible_web/controllers/api/stats_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -389,16 +389,16 @@ defmodule PlausibleWeb.Api.StatsController do
page_filter? =
Filters.filtering_on_dimension?(query, "event:page", behavioral_filters: :ignore)

scroll_depth_enabled? = scroll_depth_enabled?(site, current_user)
include_scroll_depth? = Plausible.Stats.ScrollDepth.feature_visible?(site, current_user)

metrics = [:visitors, :visits, :pageviews, :sample_percent]

metrics =
cond do
page_filter? && scroll_depth_enabled? && query.include_imported ->
page_filter? && include_scroll_depth? && query.include_imported ->
metrics ++ [:scroll_depth]

page_filter? && scroll_depth_enabled? ->
page_filter? && include_scroll_depth? ->
metrics ++ [:bounce_rate, :scroll_depth, :time_on_page]

page_filter? && query.include_imported ->
Expand Down Expand Up @@ -851,7 +851,7 @@ defmodule PlausibleWeb.Api.StatsController do
params = Map.put(params, "property", "event:page")
query = Query.from(site, params, debug_metadata(conn))

include_scroll_depth? = scroll_depth_enabled?(site, current_user)
include_scroll_depth? = Plausible.Stats.ScrollDepth.feature_visible?(site, current_user)

extra_metrics =
cond do
Expand Down Expand Up @@ -1663,9 +1663,4 @@ defmodule PlausibleWeb.Api.StatsController do
defp toplevel_goal_filter?(query) do
Filters.filtering_on_dimension?(query, "event:goal", max_depth: 0)
end

def scroll_depth_enabled?(site, user) do
FunWithFlags.enabled?(:scroll_depth, for: user) ||
FunWithFlags.enabled?(:scroll_depth, for: site)
end
end
18 changes: 14 additions & 4 deletions lib/plausible_web/controllers/stats_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,16 @@ defmodule PlausibleWeb.StatsController do

def stats(%{assigns: %{site: site}} = conn, _params) do
site = Plausible.Repo.preload(site, :owner)
current_user = conn.assigns[:current_user]
stats_start_date = Plausible.Sites.stats_start_date(site)
can_see_stats? = not Sites.locked?(site) or conn.assigns[:site_role] == :super_admin
demo = site.domain == PlausibleWeb.Endpoint.host()
dogfood_page_path = if demo, do: "/#{site.domain}", else: "/:dashboard"
skip_to_dashboard? = conn.params["skip_to_dashboard"] == "true"

scroll_depth_visible? =
Plausible.Stats.ScrollDepth.check_feature_visible!(site, current_user)

cond do
(stats_start_date && can_see_stats?) || (can_see_stats? && skip_to_dashboard?) ->
conn
Expand All @@ -68,11 +72,12 @@ defmodule PlausibleWeb.StatsController do
revenue_goals: list_revenue_goals(site),
funnels: list_funnels(site),
has_props: Plausible.Props.configured?(site),
scroll_depth_visible: scroll_depth_visible?,
stats_start_date: stats_start_date,
native_stats_start_date: NaiveDateTime.to_date(site.native_stats_start_at),
title: title(conn, site),
demo: demo,
flags: get_flags(conn.assigns[:current_user], site),
flags: get_flags(current_user, site),
is_dbip: is_dbip(),
dogfood_page_path: dogfood_page_path,
load_dashboard_js: true
Expand Down Expand Up @@ -193,8 +198,8 @@ defmodule PlausibleWeb.StatsController do
defp csv_graph_metrics(query, site, current_user) do
include_scroll_depth? =
!query.include_imported &&
PlausibleWeb.Api.StatsController.scroll_depth_enabled?(site, current_user) &&
Filters.filtering_on_dimension?(query, "event:page", behavioral_filters: :ignore)
Filters.filtering_on_dimension?(query, "event:page", behavioral_filters: :ignore) &&
Plausible.Stats.ScrollDepth.feature_visible?(site, current_user)

{metrics, column_headers} =
if Filters.filtering_on_dimension?(query, "event:goal", max_depth: 0) do
Expand Down Expand Up @@ -341,9 +346,13 @@ defmodule PlausibleWeb.StatsController do
defp render_shared_link(conn, shared_link) do
cond do
!shared_link.site.locked ->
current_user = conn.assigns[:current_user]
shared_link = Plausible.Repo.preload(shared_link, site: :owner)
stats_start_date = Plausible.Sites.stats_start_date(shared_link.site)

scroll_depth_visible? =
Plausible.Stats.ScrollDepth.check_feature_visible!(shared_link.site, current_user)

conn
|> put_resp_header("x-robots-tag", "noindex, nofollow")
|> delete_resp_header("x-frame-options")
Expand All @@ -353,6 +362,7 @@ defmodule PlausibleWeb.StatsController do
revenue_goals: list_revenue_goals(shared_link.site),
funnels: list_funnels(shared_link.site),
has_props: Plausible.Props.configured?(shared_link.site),
scroll_depth_visible: scroll_depth_visible?,
stats_start_date: stats_start_date,
native_stats_start_date: NaiveDateTime.to_date(shared_link.site.native_stats_start_at),
title: title(conn, shared_link.site),
Expand All @@ -362,7 +372,7 @@ defmodule PlausibleWeb.StatsController do
embedded: conn.params["embed"] == "true",
background: conn.params["background"],
theme: conn.params["theme"],
flags: get_flags(conn.assigns[:current_user], shared_link.site),
flags: get_flags(current_user, shared_link.site),
is_dbip: is_dbip(),
load_dashboard_js: true
)
Expand Down
1 change: 1 addition & 0 deletions lib/plausible_web/templates/stats/stats.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
data-revenue-goals={Jason.encode!(@revenue_goals)}
data-funnels={Jason.encode!(@funnels)}
data-has-props={to_string(@has_props)}
data-scroll-depth-visible={to_string(@scroll_depth_visible)}
data-logged-in={to_string(!!@conn.assigns[:current_user])}
data-stats-begin={@stats_start_date}
data-native-stats-begin={@native_stats_start_date}
Expand Down
14 changes: 8 additions & 6 deletions test/plausible/exports_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ defmodule Plausible.ExportsTest do
# for e2e export->import tests please see Plausible.Imported.CSVImporterTest

describe "export_queries/2" do
test "returns named ecto queries" do
queries = Plausible.Exports.export_queries(_site_id = 1, nil)
setup [:create_user, :create_site]

test "returns named ecto queries", %{site: site} do
queries = Plausible.Exports.export_queries(site.id, nil)
assert queries |> Map.values() |> Enum.all?(&match?(%Ecto.Query{}, &1))

assert Map.keys(queries) == [
Expand All @@ -24,9 +26,9 @@ defmodule Plausible.ExportsTest do
]
end

test "with date range" do
test "with date range", %{site: site} do
queries =
Plausible.Exports.export_queries(_site_id = 1, nil,
Plausible.Exports.export_queries(site.id, nil,
date_range: Date.range(~D[2023-01-01], ~D[2024-03-12])
)

Expand All @@ -44,8 +46,8 @@ defmodule Plausible.ExportsTest do
]
end

test "with custom extension" do
queries = Plausible.Exports.export_queries(_site_id = 1, nil, extname: ".ch")
test "with custom extension", %{site: site} do
queries = Plausible.Exports.export_queries(site.id, nil, extname: ".ch")

assert Map.keys(queries) == [
"imported_browsers.ch",
Expand Down
Loading

0 comments on commit 7c6ba04

Please sign in to comment.