Memoize has_stats? (#1302)

* Memoize has_stats?

* Remove unused alias

lib/plausible/auth/auth.ex

@@ -1,7 +1,6 @@
 defmodule Plausible.Auth do
   use Plausible.Repo
   alias Plausible.Auth
-  alias Plausible.Stats.Clickhouse, as: Stats
 
   def issue_email_verification(user) do
     Repo.update_all(from(c in "email_verification_codes", where: c.user_id == ^user.id),
@@ -68,7 +67,7 @@ defmodule Plausible.Auth do
   end
 
   def has_active_sites?(user, roles \\ [:owner, :admin, :viewer]) do
-    domains =
+    sites =
       Repo.all(
         from u in Plausible.Auth.User,
           where: u.id == ^user.id,
@@ -77,10 +76,10 @@ defmodule Plausible.Auth do
           where: sm.role in ^roles,
           join: s in Plausible.Site,
           on: s.id == sm.site_id,
-          select: s.domain
+          select: s
       )
 
-    Stats.has_pageviews?(domains)
+    Enum.any?(sites, &Plausible.Sites.has_stats?/1)
   end
 
   def user_owns_sites?(user) do

lib/plausible/site/schema.ex

@@ -10,6 +10,7 @@ defmodule Plausible.Site do
     field :timezone, :string, default: "Etc/UTC"
     field :public, :boolean
     field :locked, :boolean
+    field :has_stats, :boolean
 
     many_to_many :members, User, join_through: Plausible.Site.Membership
     has_many :memberships, Plausible.Site.Membership
@@ -42,6 +43,10 @@ defmodule Plausible.Site do
     change(site, public: false)
   end
 
+  def set_has_stats(site, has_stats_val) do
+    change(site, has_stats: has_stats_val)
+  end
+
   defp clean_domain(changeset) do
     clean_domain =
       (get_field(changeset, :domain) || "")

lib/plausible/sites.ex

@@ -26,6 +26,23 @@ defmodule Plausible.Sites do
     end
   end
 
+  def has_stats?(site) do
+    if site.has_stats do
+      true
+    else
+      has_stats = Plausible.Stats.Clickhouse.has_pageviews?(site)
+
+      if has_stats do
+        Plausible.Site.set_has_stats(site, true)
+        |> Repo.update()
+
+        true
+      else
+        false
+      end
+    end
+  end
+
   def create_shared_link(site, name, password \\ nil) do
     changes =
       SharedLink.changeset(

lib/plausible/stats/clickhouse.ex

@@ -892,7 +892,9 @@ defmodule Plausible.Stats.Clickhouse do
   end
 
   def has_pageviews?(site) do
-    ClickhouseRepo.exists?(from e in "events", where: e.domain == ^site.domain)
+    ClickhouseRepo.exists?(
+      from e in "events", where: e.domain == ^site.domain and e.name == "pageview"
+    )
   end
 
   def all_props(site, %Query{filters: %{"props" => meta}} = query) when is_map(meta) do

lib/plausible_web/controllers/stats_controller.ex

@@ -1,16 +1,15 @@
 defmodule PlausibleWeb.StatsController do
   use PlausibleWeb, :controller
   use Plausible.Repo
-  alias Plausible.Stats.Clickhouse, as: Stats
   alias Plausible.Stats.Query
 
   plug PlausibleWeb.AuthorizeSiteAccess when action in [:stats, :csv_export]
 
   def stats(%{assigns: %{site: site}} = conn, _params) do
-    has_pageviews = Stats.has_pageviews?(site)
+    has_stats = Plausible.Sites.has_stats?(site)
 
     cond do
-      !site.locked && has_pageviews ->
+      !site.locked && has_stats ->
         demo = site.domain == PlausibleWeb.Endpoint.host()
         offer_email_report = get_session(conn, site.domain <> "_offer_email_report")
 
@@ -26,7 +25,7 @@ defmodule PlausibleWeb.StatsController do
           demo: demo
         )
 
-      !site.locked && !has_pageviews ->
+      !site.locked && !has_stats ->
         conn
         |> assign(:skip_plausible_tracking, true)
         |> render("waiting_first_pageview.html", site: site)

lib/plausible_web/templates/stats/waiting_first_pageview.html.eex

@@ -10,7 +10,7 @@
       })
   }
 
-  setInterval(updateStatus, 1500)
+  setInterval(updateStatus, 5000)
 </script>
 
 <div class="w-full max-w-md mx-auto mt-8">

lib/workers/send_site_setup_emails.ex

@@ -2,7 +2,6 @@ defmodule Plausible.Workers.SendSiteSetupEmails do
   use Plausible.Repo
   use Oban.Worker, queue: :site_setup_emails
   require Logger
-  alias Plausible.Stats.Clickhouse, as: Stats
 
   @impl Oban.Worker
   def perform(_job) do
@@ -46,7 +45,7 @@ defmodule Plausible.Workers.SendSiteSetupEmails do
         Plausible.Sites.owner_for(site)
         |> Repo.preload(:subscription)
 
-      setup_completed = Stats.has_pageviews?(site)
+      setup_completed = Plausible.Sites.has_stats?(site)
       hours_passed = Timex.diff(Timex.now(), site.inserted_at, :hours)
 
       if !setup_completed && hours_passed > 47 do
@@ -69,7 +68,7 @@ defmodule Plausible.Workers.SendSiteSetupEmails do
         Plausible.Sites.owner_for(site)
         |> Repo.preload(:subscription)
 
-      if Stats.has_pageviews?(site) do
+      if Plausible.Sites.has_stats?(site) do
         send_setup_success_email(owner, site)
       end
     end

priv/repo/migrations/20210906102736_memoize_setup_complete.exs

@@ -0,0 +1,9 @@
+defmodule Plausible.Repo.Migrations.MemoizeSetupComplete do
+  use Ecto.Migration
+
+  def change do
+    alter table(:sites) do
+      add :has_stats, :boolean, null: false, default: false
+    end
+  end
+end

test/plausible/sites_test.exs

@@ -1,5 +1,6 @@
 defmodule Plausible.SitesTest do
   use Plausible.DataCase
+  import Plausible.TestUtils
   alias Plausible.Sites
 
   describe "is_member?" do
@@ -17,4 +18,35 @@ defmodule Plausible.SitesTest do
       refute Sites.is_member?(user.id, site)
     end
   end
+
+  describe "has_stats?" do
+    test "is false if site has no stats" do
+      site = insert(:site)
+
+      refute Sites.has_stats?(site)
+    end
+
+    test "is true if site has stats" do
+      site = insert(:site)
+
+      populate_stats(site, [
+        build(:pageview)
+      ])
+
+      assert Sites.has_stats?(site)
+    end
+
+    test "memoizes has_stats value" do
+      site = insert(:site)
+
+      populate_stats(site, [
+        build(:pageview)
+      ])
+
+      refute site.has_stats
+
+      assert Sites.has_stats?(site)
+      assert Repo.reload!(site).has_stats
+    end
+  end
 end