fix: failing tests

Author: hamir-suspect

guard/lib/guard/front_repo/service_account.ex

@@ -33,6 +33,7 @@ defmodule Guard.FrontRepo.ServiceAccount do
     )
     |> foreign_key_constraint(:id, name: :service_accounts_id_fkey)
     |> foreign_key_constraint(:creator_id, name: :service_accounts_creator_id_fkey)
+    |> unique_constraint(:id, name: :service_accounts_pkey)
   end
 
   @doc """

guard/lib/guard/front_repo/user.ex

@@ -78,6 +78,7 @@ defmodule Guard.FrontRepo.User do
       :visited_at
     ])
     |> validate_required([:email, :name])
+    |> validate_length(:name, max: 255, message: "Name cannot exceed 255 characters")
     |> validate_format(:email, ~r/^([\w\.%\+\-]+)@([\w\-]+\.)+([\w]{2,})$/i,
       message: "is not a valid email"
     )
@@ -257,6 +258,7 @@ defmodule Guard.FrontRepo.User do
       :idempotency_token
     ])
     |> validate_required([:email, :name, :creation_source, :org_id])
+    |> validate_length(:name, max: 255, message: "Name cannot exceed 255 characters")
     |> validate_inclusion(:creation_source, [:service_account])
     |> put_change(:single_org_user, true)
     |> validate_format(:email, ~r/^[\w\-\.]+@sa\.[\w\-\.]+\.semaphoreci\.com$/i,

guard/lib/guard/store/service_account.ex

@@ -149,15 +149,20 @@ defmodule Guard.Store.ServiceAccount do
   @spec delete(String.t()) :: {:ok, :deleted} | {:error, :not_found | :internal_error}
   def delete(service_account_id) when is_binary(service_account_id) do
     if valid_uuid?(service_account_id) do
-      FrontRepo.transaction(fn ->
+      case FrontRepo.transaction(fn ->
         with {:ok, current_data} <- find(service_account_id),
              {:ok, _updated_user} <- deactivate_user_record(service_account_id) do
           :deleted
         else
-          {:error, reason} ->
-            FrontRepo.rollback(reason)
+          {:error, :not_found} ->
+            FrontRepo.rollback(:not_found)
+          {:error, _reason} ->
+            FrontRepo.rollback(:internal_error)
         end
-      end)
+      end) do
+        {:ok, :deleted} -> {:ok, :deleted}
+        {:error, reason} -> {:error, reason}
+      end
     else
       {:error, :invalid_id}
     end
@@ -216,8 +221,6 @@ defmodule Guard.Store.ServiceAccount do
         description: sa.description,
         org_id: u.org_id,
         creator_id: sa.creator_id,
-        created_at: sa.created_at,
-        updated_at: sa.updated_at,
         deactivated: u.deactivated,
         email: u.email
       }
@@ -252,6 +255,8 @@ defmodule Guard.Store.ServiceAccount do
       authentication_token: hashed_token
     }
 
+    require Logger
+    Logger.info("User params: #{inspect(user_params)}")
     changeset = User.changeset(%User{}, user_params)
 
     case FrontRepo.insert(changeset) do
@@ -377,11 +382,10 @@ defmodule Guard.Store.ServiceAccount do
       description: service_account.description,
       org_id: user.org_id,
       creator_id: service_account.creator_id,
-      created_at: service_account.created_at,
-      updated_at: service_account.updated_at,
       deactivated: user.deactivated,
       user_id: user.id,
-      email: user.email
+      email: user.email,
+      user: user
     }
   end
 end

guard/priv/front_repo/migrations/20250716010415_create_service_accounts.exs

@@ -5,7 +5,7 @@ defmodule Guard.FrontRepo.Migrations.CreateServiceAccounts do
     create table(:service_accounts, primary_key: false) do
       add :id, references(:users, type: :binary_id, on_delete: :delete_all), 
           primary_key: true, null: false
-      add :description, :string
+      add :description, :string, size: 500
       add :creator_id, references(:users, type: :binary_id, on_delete: :nilify_all), 
           null: false
     end

guard/test/guard/front_repo/service_account_test.exs

@@ -9,15 +9,15 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       user = create_test_user()
 
       attrs = %{
-        user_id: user.id,
+        id: user.id,
         description: "Test service account description",
-        creator_id: Ecto.UUID.generate()
+        creator_id: user.id
       }
 
       changeset = ServiceAccount.changeset(%ServiceAccount{}, attrs)
 
       assert changeset.valid?
-      assert changeset.changes.user_id == user.id
+      assert changeset.changes.id == user.id
       assert changeset.changes.description == "Test service account description"
       assert changeset.changes.creator_id == attrs.creator_id
     end
@@ -26,21 +26,23 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       user = create_test_user()
 
       attrs = %{
-        user_id: user.id
+        id: user.id,
+        creator_id: user.id
       }
 
       changeset = ServiceAccount.changeset(%ServiceAccount{}, attrs)
 
       assert changeset.valid?
-      assert changeset.changes.user_id == user.id
+      assert changeset.changes.id == user.id
     end
 
-    test "sets id to match user_id" do
+    test "sets id to match user id" do
       user = create_test_user()
 
       attrs = %{
-        user_id: user.id,
-        description: "Test description"
+        id: user.id,
+        description: "Test description",
+        creator_id: user.id
       }
 
       changeset = ServiceAccount.changeset(%ServiceAccount{}, attrs)
@@ -49,22 +51,37 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       assert changeset.changes.id == user.id
     end
 
-    test "requires user_id field" do
+    test "requires id field" do
       attrs = %{
         description: "Test description"
       }
 
       changeset = ServiceAccount.changeset(%ServiceAccount{}, attrs)
 
       refute changeset.valid?
-      assert {"can't be blank", _} = changeset.errors[:user_id]
+      assert {"can't be blank", _} = changeset.errors[:id]
+    end
+
+    test "requires creator_id field" do
+      user = create_test_user()
+      
+      attrs = %{
+        id: user.id,
+        description: "Test description"
+      }
+
+      changeset = ServiceAccount.changeset(%ServiceAccount{}, attrs)
+
+      refute changeset.valid?
+      assert {"can't be blank", _} = changeset.errors[:creator_id]
     end
 
     test "validates description length" do
       user = create_test_user()
 
       attrs = %{
-        user_id: user.id,
+        id: user.id,
+        creator_id: user.id,
         # Exceeds 500 character limit
         description: String.duplicate("a", 501)
       }
@@ -79,7 +96,8 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       user = create_test_user()
 
       attrs = %{
-        user_id: user.id,
+        id: user.id,
+        creator_id: user.id,
         # Exactly 500 characters
         description: String.duplicate("a", 500)
       }
@@ -93,7 +111,8 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       user = create_test_user()
 
       attrs = %{
-        user_id: user.id,
+        id: user.id,
+        creator_id: user.id,
         description: ""
       }
 
@@ -106,7 +125,8 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       user = create_test_user()
 
       attrs = %{
-        user_id: user.id,
+        id: user.id,
+        creator_id: user.id,
         description: nil
       }
 
@@ -115,11 +135,13 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       assert changeset.valid?
     end
 
-    test "enforces foreign key constraint on user_id" do
+    test "enforces foreign key constraint on id" do
       non_existent_user_id = Ecto.UUID.generate()
+      creator_user = create_test_user()
 
       attrs = %{
-        user_id: non_existent_user_id,
+        id: non_existent_user_id,
+        creator_id: creator_user.id,
         description: "Test description"
       }
 
@@ -129,24 +151,27 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       assert changeset.valid?
 
       {:error, changeset} = FrontRepo.insert(changeset)
-      assert {"does not exist", _} = changeset.errors[:user_id]
+      assert {"does not exist", _} = changeset.errors[:id]
     end
 
-    test "enforces unique constraint on user_id" do
+    test "enforces unique constraint on id" do
       user = create_test_user()
+      creator_user = create_test_user()
 
       # Create first service account
       attrs1 = %{
-        user_id: user.id,
+        id: user.id,
+        creator_id: creator_user.id,
         description: "First service account"
       }
 
       changeset1 = ServiceAccount.changeset(%ServiceAccount{}, attrs1)
       {:ok, _} = FrontRepo.insert(changeset1)
 
-      # Try to create second service account with same user_id
+      # Try to create second service account with same id
       attrs2 = %{
-        user_id: user.id,
+        id: user.id,
+        creator_id: creator_user.id,
         description: "Second service account"
       }
 
@@ -156,7 +181,7 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       assert changeset2.valid?
 
       {:error, changeset} = FrontRepo.insert(changeset2)
-      assert {"has already been taken", _} = changeset.errors[:user_id]
+      assert {"has already been taken", _} = changeset.errors[:id]
     end
 
   end
@@ -204,39 +229,39 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       assert changeset.valid?
     end
 
-    test "does not allow updating user_id" do
+    test "does not allow updating id" do
       user = create_test_user()
       service_account = create_test_service_account(user)
       another_user = create_test_user()
 
       attrs = %{
-        user_id: another_user.id,
+        id: another_user.id,
         description: "Updated description"
       }
 
       changeset = ServiceAccount.update_changeset(service_account, attrs)
 
-      # user_id should not be in the changeset changes
+      # id should not be in the changeset changes
       assert changeset.valid?
-      refute Map.has_key?(changeset.changes, :user_id)
+      refute Map.has_key?(changeset.changes, :id)
       assert changeset.changes.description == "Updated description"
     end
 
-    test "does not allow updating id" do
+    test "does not allow updating creator_id" do
       user = create_test_user()
       service_account = create_test_service_account(user)
-      new_id = Ecto.UUID.generate()
+      new_creator_id = Ecto.UUID.generate()
 
       attrs = %{
-        id: new_id,
+        creator_id: new_creator_id,
         description: "Updated description"
       }
 
       changeset = ServiceAccount.update_changeset(service_account, attrs)
 
-      # id should not be in the changeset changes
+      # creator_id should not be in the changeset changes
       assert changeset.valid?
-      refute Map.has_key?(changeset.changes, :id)
+      refute Map.has_key?(changeset.changes, :creator_id)
       assert changeset.changes.description == "Updated description"
     end
   end
@@ -271,7 +296,7 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
       service_account = create_test_service_account(user)
 
       assert is_binary(service_account.id)
-      assert is_binary(service_account.user_id)
+      assert is_binary(service_account.creator_id)
       assert is_binary(service_account.description) or is_nil(service_account.description)
     end
 
@@ -280,7 +305,7 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
 
       attrs = %{
         id: user.id,
-        user_id: user.id,
+        creator_id: user.id,
         description: "Test description"
       }
 
@@ -312,10 +337,13 @@ defmodule Guard.FrontRepo.ServiceAccountTest do
   end
 
   defp create_test_service_account(user) do
+    # Create a creator user to satisfy foreign key constraint
+    creator_user = create_test_user()
+    
     attrs = %{
-      user_id: user.id,
+      id: user.id,
       description: "Test service account description",
-      creator_id: Ecto.UUID.generate()
+      creator_id: creator_user.id
     }
 
     changeset = ServiceAccount.changeset(%ServiceAccount{}, attrs)