Simplify Phoenix error template (#2913)

* Simplify Phoenix error template

* Test tracking script is not included in error pages

* Test tracking script is not rendered in error templates

* Rename error layout and remove unnecessary HTML boilerplate

* Add layout setting to errors rendered without exceptions

* Add skip_plausible_tracking option to more pages

config/config.exs

@@ -7,7 +7,7 @@ config :plausible, PlausibleWeb.Endpoint,
   pubsub_server: Plausible.PubSub,
   render_errors: [
     view: PlausibleWeb.ErrorView,
-    layout: {PlausibleWeb.LayoutView, "focus.html"},
+    layout: {PlausibleWeb.LayoutView, "base_error.html"},
     accepts: ~w(html json)
   ]
 

lib/plausible_web/controllers/billing_controller.ex

@@ -27,6 +27,7 @@ defmodule PlausibleWeb.BillingController do
 
       true ->
         render(conn, "upgrade.html",
+          skip_plausible_tracking: true,
           usage: Plausible.Billing.usage(user),
           user: user,
           layout: {PlausibleWeb.LayoutView, "focus.html"}
@@ -45,6 +46,7 @@ defmodule PlausibleWeb.BillingController do
 
       plan ->
         render(conn, "upgrade_to_plan.html",
+          skip_plausible_tracking: true,
           user: user,
           plan: plan,
           layout: {PlausibleWeb.LayoutView, "focus.html"}
@@ -56,7 +58,10 @@ defmodule PlausibleWeb.BillingController do
   end
 
   def upgrade_success(conn, _params) do
-    render(conn, "upgrade_success.html", layout: {PlausibleWeb.LayoutView, "focus.html"})
+    render(conn, "upgrade_success.html",
+      layout: {PlausibleWeb.LayoutView, "focus.html"},
+      skip_plausible_tracking: true
+    )
   end
 
   def change_plan_form(conn, _params) do
@@ -76,6 +81,7 @@ defmodule PlausibleWeb.BillingController do
       subscription && user.enterprise_plan &&
           subscription.paddle_plan_id == user.enterprise_plan.paddle_plan_id ->
         render(conn, "change_enterprise_plan_contact_us.html",
+          skip_plausible_tracking: true,
           user: user,
           plan: user.enterprise_plan,
           layout: {PlausibleWeb.LayoutView, "focus.html"}
@@ -83,6 +89,7 @@ defmodule PlausibleWeb.BillingController do
 
       subscription ->
         render(conn, "change_plan.html",
+          skip_plausible_tracking: true,
           subscription: subscription,
           layout: {PlausibleWeb.LayoutView, "focus.html"}
         )
@@ -106,6 +113,7 @@ defmodule PlausibleWeb.BillingController do
 
       true ->
         render(conn, "change_enterprise_plan.html",
+          skip_plausible_tracking: true,
           user: user,
           plan: new_plan,
           layout: {PlausibleWeb.LayoutView, "focus.html"}
@@ -117,6 +125,7 @@ defmodule PlausibleWeb.BillingController do
     with {:ok, {subscription, preview_info}} <-
            preview_subscription(conn.assigns.current_user, new_plan_id) do
       render(conn, "change_plan_preview.html",
+        skip_plausible_tracking: true,
         subscription: subscription,
         preview_info: preview_info,
         layout: {PlausibleWeb.LayoutView, "focus.html"}

lib/plausible_web/controllers/error_report_controller.ex

@@ -23,6 +23,8 @@ defmodule PlausibleWeb.ErrorReportController do
   defp thanks(conn) do
     conn
     |> put_view(PlausibleWeb.ErrorView)
-    |> render("server_error_report_thanks.html", layout: {PlausibleWeb.LayoutView, "focus.html"})
+    |> render("server_error_report_thanks.html",
+      layout: {PlausibleWeb.LayoutView, "base_error.html"}
+    )
   end
 end

lib/plausible_web/controllers/helpers.ex

@@ -6,13 +6,16 @@ defmodule PlausibleWeb.ControllerHelpers do
     conn
     |> put_status(status)
     |> put_view(PlausibleWeb.ErrorView)
-    |> render("#{status}.html", layout: false, message: message)
+    |> render("#{status}.html", message: message, layout: error_layout())
   end
 
   def render_error(conn, status) do
     conn
     |> put_status(status)
     |> put_view(PlausibleWeb.ErrorView)
-    |> render("#{status}.html", layout: false)
+    |> render("#{status}.html", layout: error_layout())
   end
+
+  defp error_layout,
+    do: Application.get_env(:plausible, PlausibleWeb.Endpoint)[:render_errors][:layout]
 end

lib/plausible_web/controllers/site_controller.ex

@@ -877,6 +877,7 @@ defmodule PlausibleWeb.SiteController do
     changeset = Plausible.Site.update_changeset(conn.assigns.site)
 
     render(conn, "change_domain.html",
+      skip_plausible_tracking: true,
       changeset: changeset,
       layout: {PlausibleWeb.LayoutView, "focus.html"}
     )
@@ -893,6 +894,7 @@ defmodule PlausibleWeb.SiteController do
 
       {:error, changeset} ->
         render(conn, "change_domain.html",
+          skip_plausible_tracking: true,
           changeset: changeset,
           layout: {PlausibleWeb.LayoutView, "focus.html"}
         )

lib/plausible_web/templates/error/error.html.eex

@@ -1,21 +0,0 @@
-<!DOCTYPE html>
-<html lang="en" class="h-full">
-  <head>
-    <meta charset="utf-8"/>
-    <meta http-equiv="X-UA-Compatible" content="IE=edge"/>
-    <meta name="viewport" content="width=device-width, initial-scale=1.0"/>
-    <meta name="description" content="A lightweight, non-intrusive alternative to Google Analytics."/>
-    <link rel="icon" type="image/png" sizes="32x32" href="<%= PlausibleWeb.Router.Helpers.static_path(@conn, "/images/icon/plausible_favicon.png") %>">
-    <title><%= assigns[:title] || "Plausible · Web analytics" %></title>
-    <link rel="stylesheet" href="<%= Routes.static_path(@conn, "/css/app.css") %>"/>
-    <script type="text/javascript" data-pref="<%= @conn.assigns[:current_user] && @conn.assigns[:current_user].theme %>" src="<%= Routes.static_path(@conn, "/js/applyTheme.js") %>"></script>
-
-  </head>
-  <body class="flex flex-col h-full bg-gray-100 dark:bg-gray-900">
-    <div class="container text-center mt-24">
-      <h1 class="text-5xl font-black dark:text-gray-100"><%= @status %></h1>
-      <div class="my-4 text-xl dark:text-gray-100"><%= @message %></div>
-      <%= link("Go to the homepage", to: PlausibleWeb.LayoutView.home_dest(@conn), class: "button mt-4") %>
-    </div>
-  </body>
-</html>

lib/plausible_web/templates/error/generic_error.html.eex

@@ -0,0 +1,5 @@
+<div class="container text-center mt-24">
+  <h1 class="text-5xl font-black dark:text-gray-100"><%= @status %></h1>
+  <div class="my-4 text-xl dark:text-gray-100"><%= @message %></div>
+  <%= link("Go to the homepage", to: PlausibleWeb.LayoutView.home_dest(@conn), class: "button mt-4") %>
+</div>

lib/plausible_web/templates/error/server_error.html.eex

@@ -50,7 +50,5 @@
       </p>
       </div>
     <% end %>
-
-
   </div>
 </div>

lib/plausible_web/templates/layout/_tracking.html.eex

@@ -2,9 +2,7 @@
   <%= if Application.get_env(:plausible, :environment) in ["prod", "staging"] do %>
     <script defer data-domain="<%= base_domain() %>" event-logged_in="<%= !!@conn.assigns[:current_user] %>" src="<%="#{plausible_url()}/js/script.pageview-props.js"%>"></script>
     <script>window.plausible = window.plausible || function() { (window.plausible.q = window.plausible.q || []).push(arguments) }</script>
-  <% end %>
-
-  <%= if Application.get_env(:plausible, :environment) == "dev" do %>
+  <% else %>
     <script defer data-domain="<%= base_domain() %>" event-logged_in="<%= !!@conn.assigns[:current_user] %>" src="<%="#{plausible_url()}/js/plausible.local.pageview-props.js"%>"></script>
     <script>window.plausible = window.plausible || function() { (window.plausible.q = window.plausible.q || []).push(arguments) }</script>
   <% end %>

lib/plausible_web/templates/layout/base_error.html.eex

@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html lang="en" class="h-full">
+  <head>
+    <meta charset="utf-8"/>
+    <meta http-equiv="X-UA-Compatible" content="IE=edge"/>
+    <meta name="viewport" content="width=device-width, initial-scale=1.0"/>
+    <link rel="icon" type="image/png" sizes="32x32" href="<%= PlausibleWeb.Router.Helpers.static_path(PlausibleWeb.Endpoint, "/images/icon/plausible_favicon.png") %>">
+    <title>Plausible · Web analytics</title>
+    <link rel="stylesheet" href="<%= Routes.static_path(PlausibleWeb.Endpoint, "/css/app.css") %>"/>
+  </head>
+  <body class="flex flex-col h-full bg-gray-100 dark:bg-gray-900">
+    <div class="w-full my-8 text-center">
+      <%= img_tag(PlausibleWeb.Router.Helpers.static_path(PlausibleWeb.Endpoint, "/images/icon/plausible_logo_dark.png"), class: "hidden dark:inline", style: "height: 2.5rem;", alt: "Plausible logo") %>
+      <%= img_tag(PlausibleWeb.Router.Helpers.static_path(PlausibleWeb.Endpoint, "/images/icon/plausible_logo.png"), class: "inline dark:hidden", style: "height: 2.5rem;", alt: "Plausible logo")%>
+    </div>
+
+    <%= @inner_content %>
+
+    <script type="text/javascript" src="<%= Routes.static_path(PlausibleWeb.Endpoint, "/js/app.js") %>"></script>
+  </body>
+</html>

lib/plausible_web/views/error_view.ex

@@ -9,17 +9,12 @@ defmodule PlausibleWeb.ErrorView do
   end
 
   def render("404.html", assigns) do
-    render(
-      "error.html",
-      Map.merge(
-        %{
-          layout: false,
-          status: 404,
-          message: "Oops! There's nothing here"
-        },
+    assigns =
       assigns
-      )
-    )
+      |> Map.put(:status, 404)
+      |> Map.put_new(:message, "Oops! There's nothing here")
+
+    render("generic_error.html", assigns)
   end
 
   def render(<<"5", _error_5xx::binary-size(2), ".html">>, assigns) do
@@ -45,18 +40,11 @@ defmodule PlausibleWeb.ErrorView do
   end
 
   def template_not_found(template, assigns) do
-    status = String.trim_trailing(template, ".html")
-
-    render(
-      "error.html",
-      Map.merge(
-        %{
-          layout: false,
-          status: status,
-          message: Phoenix.Controller.status_message_from_template(template)
-        },
+    assigns =
       assigns
-      )
-    )
+      |> Map.put_new(:message, Phoenix.Controller.status_message_from_template(template))
+      |> Map.put(:status, String.trim_trailing(template, ".html"))
+
+    render("generic_error.html", assigns)
   end
 end

test/plausible_web/views/error_view_test.exs

@@ -0,0 +1,13 @@
+defmodule PlausibleWeb.ErrorViewTest do
+  use PlausibleWeb.ConnCase, async: false
+
+  test "renders 500.html", %{conn: conn} do
+    conn = get(conn, "/test")
+    layout = Application.get_env(:plausible, PlausibleWeb.Endpoint)[:render_errors][:layout]
+
+    error_html =
+      Phoenix.View.render_to_string(PlausibleWeb.ErrorView, "500.html", conn: conn, layout: layout)
+
+    refute error_html =~ "data-domain="
+  end
+end