Improve forms (#3380)

* Make client-facing user changesets accept only editable fields

* Add controller test

lib/plausible/auth/auth.ex

@@ -44,7 +44,7 @@ defmodule Plausible.Auth do
           Ecto.Multi.new()
           |> Ecto.Multi.update(
             :user,
-            Plausible.Auth.User.changeset(user, %{email_verified: true})
+            Ecto.Changeset.change(user, email_verified: true)
           )
           |> Ecto.Multi.update_all(
             :codes,

lib/plausible/auth/user.ex

@@ -26,7 +26,7 @@ defmodule Plausible.Auth.User do
     field :name, :string
     field :last_seen, :naive_datetime
     field :trial_expiry_date, :date
-    field :theme, :string
+    field :theme, Ecto.Enum, values: [:system, :light, :dark]
     field :email_verified, :boolean
     embeds_one :grace_period, Plausible.Auth.GracePeriod, on_replace: :update
 
@@ -54,6 +54,13 @@ defmodule Plausible.Auth.User do
     |> unique_constraint(:email)
   end
 
+  def settings_changeset(user, attrs \\ %{}) do
+    user
+    |> cast(attrs, [:email, :name, :theme])
+    |> validate_required([:email, :name, :theme])
+    |> unique_constraint(:email)
+  end
+
   def changeset(user, attrs \\ %{}) do
     user
     |> cast(attrs, [:email, :name, :email_verified, :theme, :trial_expiry_date])

lib/plausible_web/controllers/auth_controller.ex

@@ -331,12 +331,12 @@ defmodule PlausibleWeb.AuthController do
   end
 
   def user_settings(conn, _params) do
-    changeset = Auth.User.changeset(conn.assigns[:current_user])
+    changeset = Auth.User.settings_changeset(conn.assigns[:current_user])
     render_settings(conn, changeset)
   end
 
   def save_settings(conn, %{"user" => user_params}) do
-    changes = Auth.User.changeset(conn.assigns[:current_user], user_params)
+    changes = Auth.User.settings_changeset(conn.assigns[:current_user], user_params)
 
     case Repo.update(changes) do
       {:ok, _user} ->

lib/plausible_web/live/register_form.ex

@@ -27,9 +27,9 @@ defmodule PlausibleWeb.Live.RegisterForm do
     else
       changeset =
         if invitation = socket.assigns.invitation do
-          Auth.User.changeset(%Auth.User{email: invitation.email})
+          Auth.User.settings_changeset(%Auth.User{email: invitation.email})
         else
-          Auth.User.changeset(%Auth.User{})
+          Auth.User.settings_changeset(%Auth.User{})
         end
 
       {:ok,

lib/plausible_web/live/reset_password_form.ex

@@ -19,7 +19,7 @@ defmodule PlausibleWeb.Live.ResetPasswordForm do
         Repo.get_by!(Auth.User, email: email)
       end)
 
-    changeset = Auth.User.changeset(socket.assigns.user)
+    changeset = Auth.User.settings_changeset(socket.assigns.user)
 
     {:ok,
      assign(socket,

test/plausible/auth/user_test.exs

@@ -3,6 +3,68 @@ defmodule Plausible.Auth.UserTest do
 
   alias Plausible.Auth.User
 
+  describe "settings_changeset/2" do
+    test "fails for empty input and user" do
+      changeset = User.settings_changeset(%User{}, %{})
+
+      refute changeset.valid?
+    end
+
+    test "succeeds for non-empty user even if input is empty" do
+      changeset =
+        User.settings_changeset(
+          %User{name: "Mary Jane", email: "mary@plausible.test", theme: "system"},
+          %{}
+        )
+
+      assert changeset.valid?
+    end
+
+    test "succeeds for valid user details input" do
+      changeset =
+        User.settings_changeset(
+          %User{name: "Mary Jane", email: "mary@plausible.test", theme: "system"},
+          %{name: "Tony Mangle", email: "tony@plausible.test"}
+        )
+
+      assert changeset.valid?
+      assert changeset.changes == %{name: "Tony Mangle", email: "tony@plausible.test"}
+    end
+
+    test "succeeds for valid theme input" do
+      changeset =
+        User.settings_changeset(
+          %User{name: "Mary Jane", email: "mary@plausible.test", theme: "system"},
+          %{theme: "dark"}
+        )
+
+      assert changeset.valid?
+      assert changeset.changes == %{theme: :dark}
+    end
+
+    test "fails on invalid user details input" do
+      changeset =
+        User.settings_changeset(
+          %User{name: "Mary Jane", email: "mary@plausible.test", theme: "system"},
+          %{name: ""}
+        )
+
+      refute changeset.valid?
+      assert changeset.errors[:name]
+    end
+
+    test "fails on invalid theme input" do
+      changeset =
+        User.settings_changeset(
+          %User{name: "Mary Jane", email: "mary@plausible.test", theme: "system"},
+          %{theme: "invalid"}
+        )
+
+      refute changeset.valid?
+      assert changeset.errors[:theme]
+    end
+  end
+
   describe "password_strength/1" do
     test "scores password with all arguments in changes" do
       assert %{score: score, warning: warning, suggestions: suggestions} =

test/plausible_web/controllers/auth_controller_test.exs

@@ -546,6 +546,18 @@ defmodule PlausibleWeb.AuthControllerTest do
       assert user.name == "New name"
     end
 
+    test "does not allow setting non-profile fields", %{conn: conn, user: user} do
+      expiry_date = user.trial_expiry_date
+
+      assert %Date{} = expiry_date
+
+      put(conn, "/settings", %{
+        "user" => %{"name" => "New name", "trial_expiry_date" => "2023-07-14"}
+      })
+
+      assert Repo.reload!(user).trial_expiry_date == expiry_date
+    end
+
     test "redirects user to /settings", %{conn: conn} do
       conn = put(conn, "/settings", %{"user" => %{"name" => "New name"}})