From 33ba5d79b635f44041113680e7666e798f36a481 Mon Sep 17 00:00:00 2001
From: Uku Taht <Uku.taht@gmail.com>
Date: Mon, 6 Sep 2021 13:54:51 +0300
Subject: [PATCH] Memoize has_stats? (#1302)
* Memoize has_stats?
* Remove unused alias
---
lib/plausible/auth/auth.ex | 7 ++--
lib/plausible/site/schema.ex | 5 +++
lib/plausible/sites.ex | 17 ++++++++++
lib/plausible/stats/clickhouse.ex | 4 ++-
.../controllers/stats_controller.ex | 7 ++--
.../stats/waiting_first_pageview.html.eex | 2 +-
lib/workers/send_site_setup_emails.ex | 5 ++-
.../20210906102736_memoize_setup_complete.exs | 9 ++++++
test/plausible/sites_test.exs | 32 +++++++++++++++++++
9 files changed, 75 insertions(+), 13 deletions(-)
create mode 100644 priv/repo/migrations/20210906102736_memoize_setup_complete.exs
diff --git a/lib/plausible/auth/auth.ex b/lib/plausible/auth/auth.ex
index ac3c7864..3dd82504 100644
--- a/lib/plausible/auth/auth.ex
+++ b/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
diff --git a/lib/plausible/site/schema.ex b/lib/plausible/site/schema.ex
index ade0ffc2..625c5bcd 100644
--- a/lib/plausible/site/schema.ex
+++ b/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) || "")
diff --git a/lib/plausible/sites.ex b/lib/plausible/sites.ex
index f50f034d..e073fc7f 100644
--- a/lib/plausible/sites.ex
+++ b/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(
diff --git a/lib/plausible/stats/clickhouse.ex b/lib/plausible/stats/clickhouse.ex
index 60607f6a..ac5b2a67 100644
--- a/lib/plausible/stats/clickhouse.ex
+++ b/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
diff --git a/lib/plausible_web/controllers/stats_controller.ex b/lib/plausible_web/controllers/stats_controller.ex
index cdd2023f..00c0e9ae 100644
--- a/lib/plausible_web/controllers/stats_controller.ex
+++ b/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)
diff --git a/lib/plausible_web/templates/stats/waiting_first_pageview.html.eex b/lib/plausible_web/templates/stats/waiting_first_pageview.html.eex
index b262ca0b..e66ebf68 100644
--- a/lib/plausible_web/templates/stats/waiting_first_pageview.html.eex
+++ b/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">
diff --git a/lib/workers/send_site_setup_emails.ex b/lib/workers/send_site_setup_emails.ex
index 74c95546..d4250aac 100644
--- a/lib/workers/send_site_setup_emails.ex
+++ b/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
diff --git a/priv/repo/migrations/20210906102736_memoize_setup_complete.exs b/priv/repo/migrations/20210906102736_memoize_setup_complete.exs
new file mode 100644
index 00000000..a1b5ebc0
--- /dev/null
+++ b/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
diff --git a/test/plausible/sites_test.exs b/test/plausible/sites_test.exs
index b4f4be59..077c694d 100644
--- a/test/plausible/sites_test.exs
+++ b/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
--
GitLab