Remove client names unique constraint (#2982)

Closes #2980

elixir/apps/api/lib/api/sockets.ex

@@ -3,6 +3,7 @@ defmodule API.Sockets do
   This module provides a set of helper function for Phoenix sockets and
   error handling around them.
   """
+  require Logger
 
   def options do
     [
@@ -25,12 +26,28 @@ defmodule API.Sockets do
   def handle_error(conn, :unauthenticated),
     do: Plug.Conn.send_resp(conn, 403, "Forbidden")
 
-  def handle_error(conn, %Ecto.Changeset{}),
-    do: Plug.Conn.send_resp(conn, 422, "Invalid or missing connection parameters")
+  def handle_error(conn, %Ecto.Changeset{} = changeset) do
+    Logger.error("Invalid connection request", changeset: inspect(changeset))
+    errors = changeset_error_to_string(changeset)
+    Plug.Conn.send_resp(conn, 422, "Invalid or missing connection parameters: #{errors}")
+  end
 
   def handle_error(conn, :rate_limit),
     do: Plug.Conn.send_resp(conn, 429, "Too many requests")
 
+  @doc false
+  def changeset_error_to_string(changeset) do
+    Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} ->
+      Enum.reduce(opts, msg, fn {key, value}, acc ->
+        String.replace(acc, "%{#{key}}", to_string(value))
+      end)
+    end)
+    |> Enum.reduce("", fn {k, v}, acc ->
+      joined_errors = Enum.join(v, "; ")
+      "#{acc}#{k}: #{joined_errors}\n"
+    end)
+  end
+
   def real_ip(x_headers, peer_data) do
     real_ip =
       if is_list(x_headers) and length(x_headers) > 0 do

elixir/apps/api/test/api/client/socket_test.exs

@@ -30,6 +30,19 @@ defmodule API.Client.SocketTest do
       assert connect(Socket, attrs, connect_info: @connect_info) == {:error, :invalid_token}
     end
 
+    test "renders error on invalid attrs" do
+      subject = Fixtures.Auth.create_subject()
+      {:ok, token} = Auth.create_session_token_from_subject(subject)
+
+      attrs = %{token: token}
+
+      assert {:error, changeset} = connect(Socket, attrs, connect_info: connect_info(subject))
+
+      errors = API.Sockets.changeset_error_to_string(changeset)
+      assert errors =~ "public_key: can't be blank"
+      assert errors =~ "external_id: can't be blank"
+    end
+
     test "creates a new client" do
       subject = Fixtures.Auth.create_subject()
       {:ok, token} = Auth.create_session_token_from_subject(subject)

elixir/apps/domain/lib/domain/clients/client/changeset.ex

@@ -4,7 +4,8 @@ defmodule Domain.Clients.Client.Changeset do
   alias Domain.Clients
 
   @upsert_fields ~w[external_id name public_key]a
-  @conflict_replace_fields ~w[public_key
+  @conflict_replace_fields ~w[name
+                              public_key
                               last_seen_user_agent
                               last_seen_remote_ip
                               last_seen_remote_ip_location_region
@@ -78,7 +79,6 @@ defmodule Domain.Clients.Client.Changeset do
     |> unique_constraint([:actor_id, :name])
     |> unique_constraint([:actor_id, :public_key])
     |> unique_constraint(:external_id)
-    |> unique_constraint(:name, name: :clients_account_id_actor_id_name_index)
   end
 
   defp put_client_version(changeset) do

elixir/apps/domain/priv/repo/migrations/20231221160102_drop_clients_name_unique_constraint.exs

@@ -0,0 +1,7 @@
+defmodule Domain.Repo.Migrations.DropClientsNameUniqueConstraint do
+  use Ecto.Migration
+
+  def change do
+    execute("DROP INDEX clients_account_id_actor_id_name_index")
+  end
+end

elixir/apps/domain/test/domain/clients_test.exs

@@ -395,7 +395,7 @@ defmodule Domain.ClientsTest do
 
       assert Repo.aggregate(Clients.Client, :count, :id) == 1
 
-      assert updated_client.name
+      assert updated_client.name != client.name
       assert updated_client.last_seen_remote_ip.address == subject.context.remote_ip
       assert updated_client.last_seen_remote_ip != client.last_seen_remote_ip
       assert updated_client.last_seen_user_agent == subject.context.user_agent

elixir/apps/web/test/web/live/clients/edit_test.exs

@@ -110,13 +110,11 @@ defmodule Web.Live.Clients.EditTest do
 
   test "renders changeset errors on submit", %{
     account: account,
-    actor: actor,
     identity: identity,
     client: client,
     conn: conn
   } do
-    other_client = Fixtures.Clients.create_client(account: account, actor: actor)
-    attrs = %{name: other_client.name}
+    attrs = %{name: String.duplicate("a", 256)}
 
     {:ok, lv, _html} =
       conn
@@ -127,7 +125,7 @@ defmodule Web.Live.Clients.EditTest do
            |> form("form", client: attrs)
            |> render_submit()
            |> form_validation_errors() == %{
-             "client[name]" => ["has already been taken"]
+             "client[name]" => ["should be at most 255 character(s)"]
            }
   end