Revert "Scroll depth: update site.scroll_depth_visible_at in ingest…

…ion (#5035)"

This reverts commit 84aea97.

lib/plausible/application.ex

@@ -120,7 +120,6 @@ defmodule Plausible.Application do
     setup_geolocation()
     Location.load_all()
     Plausible.Ingestion.Source.init()
-    Plausible.Ingestion.ScrollDepthVisibleAt.init()
     Plausible.Geo.await_loader()
 
     Supervisor.start_link(List.flatten(children), opts)

lib/plausible/exports.ex

@@ -418,6 +418,8 @@ 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)
 
+    include_scroll_depth? = Plausible.Stats.ScrollDepth.check_feature_visible!(site, current_user)
+
     base_q =
       from(e in sampled("events_v2"),
         where: ^export_filter(site_id, date_range),
@@ -426,7 +428,7 @@ defmodule Plausible.Exports do
         order_by: selected_as(:date)
       )
 
-    if Plausible.Stats.ScrollDepth.feature_visible?(site, current_user) do
+    if include_scroll_depth? do
       max_scroll_depth_per_visitor_q =
         from(e in "events_v2",
           where: ^export_filter(site_id, date_range),

lib/plausible/ingestion/event.ex

@@ -130,8 +130,7 @@ defmodule Plausible.Ingestion.Event do
       put_user_id: &put_user_id/2,
       validate_clickhouse_event: &validate_clickhouse_event/2,
       register_session: &register_session/2,
-      write_to_buffer: &write_to_buffer/2,
-      register_scroll_depth_visible: &register_scroll_depth_visible/2
+      write_to_buffer: &write_to_buffer/2
     ]
   end
 
@@ -406,15 +405,6 @@ defmodule Plausible.Ingestion.Event do
     event
   end
 
-  defp register_scroll_depth_visible(%__MODULE__{clickhouse_event: ev} = event, _context) do
-    if is_nil(event.site.scroll_depth_visible_at) and ev.name in ["pageleave", "engagement"] and
-         not is_nil(ev.scroll_depth) and ev.scroll_depth >= 0 and ev.scroll_depth <= 100 do
-      Plausible.Ingestion.ScrollDepthVisibleAt.mark_scroll_depth_visible(event.site.id)
-    end
-
-    event
-  end
-
   @click_id_params ["gclid", "gbraid", "wbraid", "msclkid", "fbclid", "twclid"]
 
   defp get_click_id_param(nil), do: nil

lib/plausible/site/cache.ex

@@ -31,7 +31,6 @@ defmodule Plausible.Site.Cache do
     domain_changed_from
     ingest_rate_limit_scale_seconds
     ingest_rate_limit_threshold
-    scroll_depth_visible_at
    )a
 
   @impl true

lib/plausible/stats/scroll_depth.ex

@@ -3,6 +3,11 @@ defmodule Plausible.Stats.ScrollDepth do
   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)
@@ -11,4 +16,48 @@ defmodule Plausible.Stats.ScrollDepth do
   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

lib/plausible_web/controllers/stats_controller.ex

@@ -45,7 +45,7 @@ defmodule PlausibleWeb.StatsController do
   use Plausible.Repo
 
   alias Plausible.Sites
-  alias Plausible.Stats.{Filters, Query, ScrollDepth}
+  alias Plausible.Stats.{Filters, Query}
   alias PlausibleWeb.Api
 
   plug(PlausibleWeb.Plugs.AuthorizeSiteAccess when action in [:stats, :csv_export])
@@ -59,6 +59,9 @@ defmodule PlausibleWeb.StatsController do
     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
@@ -69,7 +72,7 @@ defmodule PlausibleWeb.StatsController do
           revenue_goals: list_revenue_goals(site),
           funnels: list_funnels(site),
           has_props: Plausible.Props.configured?(site),
-          scroll_depth_visible: ScrollDepth.feature_visible?(site, current_user),
+          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),
@@ -347,6 +350,9 @@ defmodule PlausibleWeb.StatsController do
         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")
@@ -356,7 +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: ScrollDepth.feature_visible?(shared_link.site, current_user),
+          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),

test/plausible/imported/csv_importer_test.exs

@@ -1065,7 +1065,6 @@ defmodule Plausible.Imported.CSVImporterTest do
         |> Enum.map(fn event -> Map.put(event, :hostname, "csv.test") end)
 
       populate_stats(exported_site, stats)
-      Plausible.Sites.set_scroll_depth_visible_at(exported_site)
 
       initial_context = %{
         user: user,
@@ -1082,6 +1081,9 @@ defmodule Plausible.Imported.CSVImporterTest do
         |> upload_csvs()
         |> run_import()
 
+      assert %NaiveDateTime{} =
+               Plausible.Repo.reload!(exported_site).scroll_depth_visible_at
+
       assert %SiteImport{
                start_date: start_date,
                end_date: end_date,

test/plausible_web/controllers/api/external_controller_test.exs

@@ -3,8 +3,6 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
   use Plausible.ClickhouseRepo
   use Plausible.Teams.Test
 
-  alias Plausible.Repo
-
   @user_agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Safari/537.36"
   @user_agent_mobile "Mozilla/5.0 (Linux; Android 6.0; U007 Pro Build/MRA58K; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/44.0.2403.119 Mobile Safari/537.36"
   @user_agent_tablet "Mozilla/5.0 (Linux; U; Android 4.2.2; it-it; Surfing TAB B 9.7 3G Build/JDQ39) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30"
@@ -1293,15 +1291,13 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
       pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))
 
       assert pageleave.scroll_depth == 255
-      assert not scroll_depth_visible_at_set?(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)
-      assert not scroll_depth_visible_at_set?(site)
     end
 
     test "ingests valid scroll_depth for a pageleave", %{conn: conn, site: site} do
@@ -1311,7 +1307,6 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
       pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))
 
       assert pageleave.scroll_depth == 25
-      assert scroll_depth_visible_at_set?(site)
     end
 
     test "ingests scroll_depth as 100 when sd > 100", %{conn: conn, site: site} do
@@ -1321,7 +1316,6 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
       pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))
 
       assert pageleave.scroll_depth == 100
-      assert scroll_depth_visible_at_set?(site)
     end
 
     test "ingests scroll_depth as 255 when sd is a string", %{conn: conn, site: site} do
@@ -1331,7 +1325,6 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
       pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))
 
       assert pageleave.scroll_depth == 255
-      assert not scroll_depth_visible_at_set?(site)
     end
 
     test "ingests scroll_depth as 255 when sd is a negative integer", %{conn: conn, site: site} do
@@ -1341,7 +1334,6 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
       pageleave = get_events(site) |> Enum.find(&(&1.name == "pageleave"))
 
       assert pageleave.scroll_depth == 255
-      assert not scroll_depth_visible_at_set?(site)
     end
 
     test "ingests valid scroll_depth for a engagement event", %{conn: conn, site: site} do
@@ -1351,23 +1343,6 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
       event = get_events(site) |> Enum.find(&(&1.name == "engagement"))
 
       assert event.scroll_depth == 25
-      assert scroll_depth_visible_at_set?(site)
-    end
-
-    test "does not update scroll_depth_visible_at twice", %{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})
-      Plausible.Event.WriteBuffer.flush()
-
-      site1 = Repo.reload!(site)
-      assert not is_nil(site1.scroll_depth_visible_at)
-
-      post(conn, "/api/event", %{n: "pageleave", u: "https://test.com", d: site.domain, sd: 50})
-      Plausible.Event.WriteBuffer.flush()
-
-      site2 = Repo.reload!(site)
-      assert not is_nil(site2.scroll_depth_visible_at)
-      assert site1.scroll_depth_visible_at == site2.scroll_depth_visible_at
     end
   end
 
@@ -3609,9 +3584,4 @@ defmodule PlausibleWeb.Api.ExternalControllerTest do
       )
     )
   end
-
-  defp scroll_depth_visible_at_set?(site) do
-    site = Repo.reload!(site)
-    not is_nil(site.scroll_depth_visible_at)
-  end
 end

test/plausible_web/controllers/stats_controller_test.exs

@@ -85,15 +85,23 @@ defmodule PlausibleWeb.StatsControllerTest do
 
       populate_stats(site, [build(:pageview)])
 
+      # No pageleaves yet - `scroll_depth_visible_at` will remain `nil`
       html =
         conn
         |> get("/#{site.domain}")
         |> html_response(200)
 
       assert text_of_attr(html, @react_container, "data-scroll-depth-visible") == "false"
 
-      Plausible.Sites.set_scroll_depth_visible_at(site)
+      site = Repo.reload!(site)
+      assert is_nil(site.scroll_depth_visible_at)
 
+      populate_stats(site, [
+        build(:pageview, user_id: 123),
+        build(:pageleave, user_id: 123, scroll_depth: 20)
+      ])
+
+      # Pageleaves exist now - `scroll_depth_visible_at` gets set to `utc_now`
       html =
         conn
         |> get("/#{site.domain}")
@@ -1090,15 +1098,23 @@ defmodule PlausibleWeb.StatsControllerTest do
       site = insert(:site)
       link = insert(:shared_link, site: site)
 
+      # No pageleaves yet - `scroll_depth_visible_at` will remain `nil`
       html =
         conn
         |> get("/share/#{site.domain}/?auth=#{link.slug}")
         |> html_response(200)
 
       assert text_of_attr(html, @react_container, "data-scroll-depth-visible") == "false"
 
-      Plausible.Sites.set_scroll_depth_visible_at(site)
+      site = Repo.reload!(site)
+      assert is_nil(site.scroll_depth_visible_at)
+
+      populate_stats(site, [
+        build(:pageview, user_id: 123),
+        build(:pageleave, user_id: 123, scroll_depth: 20)
+      ])
 
+      # Pageleaves exist now - `scroll_depth_visible_at` gets set to `utc_now`
       html =
         conn
         |> get("/share/#{site.domain}/?auth=#{link.slug}")