fix: replace Repo.rollback with proper error handling for Oban compatibility (#201)

- Replace manual Repo.rollback/1 calls with raise/return patterns
- Fix typo: Repo.transact -> Repo.transaction
- Add unwrap_transaction_result/1 helper to reduce nesting depth
- Extract helper functions to satisfy Credo nesting depth requirements
- Fixes 'operation :rollback is rolling back unexpectedly' error

When Oban Pro's Smart engine uses Ecto.Multi for nested transactions,
manual Repo.rollback/1 calls break the transaction stack. This fix uses
proper error handling patterns:

1. Raise exceptions to trigger automatic rollback (admin.ex)
2. Return error tuples and unwrap with helper function (accounts.ex, mobile_sessions.ex)

Files changed:
- lib/towerops/admin.ex (3 functions: delete_user, delete_organization, update_billing_overrides)
- lib/towerops/accounts.ex (5 functions with helpers: update_user_email, reset_user_password, update_user_and_delete_all_tokens, do_update_user_and_delete_tokens, delete_account)
- lib/towerops/mobile_sessions.ex (2 functions with helper: complete_qr_login, do_complete_qr_login)

All tests passing (277 tests total).

Reviewed-on: graham/towerops-web#201

lib/towerops/accounts.ex

@@ -776,7 +776,7 @@ defmodule Towerops.Accounts do
     old_email = user.email
 
     result =
-      Repo.transact(fn ->
+      fn ->
         with {:ok, query} <- UserToken.verify_change_email_token_query(token, context),
              %UserToken{sent_to: email} <- Repo.one(query),
              {:ok, updated_user} <- Repo.update(User.email_changeset(user, %{email: email})),
@@ -786,7 +786,9 @@ defmodule Towerops.Accounts do
         else
           _ -> {:error, :transaction_aborted}
         end
-      end)
+      end
+      |> Repo.transaction()
+      |> unwrap_transaction_result()
 
     # Log email change for security monitoring
     case result do
@@ -904,15 +906,21 @@ defmodule Towerops.Accounts do
 
   """
   def reset_user_password(user, attrs) do
-    Repo.transact(fn ->
+    fn ->
       changeset = User.password_changeset(user, attrs)
 
-      with {:ok, {updated_user, expired_tokens}} <- update_user_and_delete_all_tokens(changeset) do
-        # Delete all reset_password tokens after successful password update
-        Repo.delete_all(from(t in UserToken, where: t.user_id == ^user.id and t.context == "reset_password"))
-        {:ok, {updated_user, expired_tokens}}
+      case update_user_and_delete_all_tokens(changeset) do
+        {:ok, {updated_user, expired_tokens}} ->
+          # Delete all reset_password tokens after successful password update
+          Repo.delete_all(from(t in UserToken, where: t.user_id == ^user.id and t.context == "reset_password"))
+          {:ok, {updated_user, expired_tokens}}
+
+        {:error, changeset} ->
+          {:error, changeset}
       end
-    end)
+    end
+    |> Repo.transaction()
+    |> unwrap_transaction_result()
   end
 
   ## Session
@@ -1096,16 +1104,34 @@ defmodule Towerops.Accounts do
 
   ## Token helper
 
+  # Helper function to unwrap nested transaction results
+  defp unwrap_transaction_result(result) do
+    case result do
+      {:ok, {:ok, value}} -> {:ok, value}
+      {:ok, {:error, reason}} -> {:error, reason}
+      {:error, reason} -> {:error, reason}
+    end
+  end
+
   defp update_user_and_delete_all_tokens(changeset) do
-    Repo.transact(fn ->
-      with {:ok, user} <- Repo.update(changeset) do
-        tokens_to_expire = Repo.all_by(UserToken, user_id: user.id)
+    changeset
+    |> do_update_user_and_delete_tokens()
+    |> Repo.transaction()
+    |> unwrap_transaction_result()
+  end
 
-        Repo.delete_all(from(t in UserToken, where: t.id in ^Enum.map(tokens_to_expire, & &1.id)))
+  defp do_update_user_and_delete_tokens(changeset) do
+    fn ->
+      case Repo.update(changeset) do
+        {:ok, user} ->
+          tokens_to_expire = Repo.all_by(UserToken, user_id: user.id)
+          Repo.delete_all(from(t in UserToken, where: t.id in ^Enum.map(tokens_to_expire, & &1.id)))
+          {:ok, {user, tokens_to_expire}}
 
-        {:ok, {user, tokens_to_expire}}
+        {:error, changeset} ->
+          {:error, changeset}
       end
-    end)
+    end
   end
 
   ## User Consent
@@ -1854,7 +1880,8 @@ defmodule Towerops.Accounts do
 
         {:error, changeset} ->
           Logger.error("Failed to delete user account: #{inspect(changeset)}")
-          Repo.rollback(changeset)
+          # Raise to trigger rollback - never call Repo.rollback/1 directly
+          raise Ecto.InvalidChangesetError, action: :delete, changeset: changeset
       end
     end)
   end

lib/towerops/admin.ex

@@ -111,7 +111,8 @@ defmodule Towerops.Admin do
 
         {:error, changeset} ->
           Logger.error("Failed to delete user: #{inspect(changeset)}")
-          Repo.rollback(changeset)
+          # Raise to trigger rollback - never call Repo.rollback/1 directly
+          raise Ecto.InvalidChangesetError, action: :delete, changeset: changeset
       end
     end)
   end
@@ -206,7 +207,8 @@ defmodule Towerops.Admin do
 
         {:error, changeset} ->
           Logger.error("Failed to delete organization: #{inspect(changeset)}")
-          Repo.rollback(changeset)
+          # Raise to trigger rollback - never call Repo.rollback/1 directly
+          raise Ecto.InvalidChangesetError, action: :delete, changeset: changeset
       end
     end)
   end
@@ -247,7 +249,8 @@ defmodule Towerops.Admin do
 
         {:error, changeset} ->
           Logger.error("Failed to update billing overrides: #{inspect(changeset.errors)}")
-          Repo.rollback(changeset)
+          # Raise to trigger rollback - never call Repo.rollback/1 directly
+          raise Ecto.InvalidChangesetError, action: :update, changeset: changeset
       end
     end)
   end

lib/towerops/mobile_sessions.ex

@@ -173,7 +173,14 @@ defmodule Towerops.MobileSessions do
   Returns {:ok, mobile_session} on success.
   """
   def complete_qr_login(token, device_attrs) do
-    Repo.transaction(fn ->
+    token
+    |> do_complete_qr_login(device_attrs)
+    |> Repo.transaction()
+    |> unwrap_transaction_result()
+  end
+
+  defp do_complete_qr_login(token, device_attrs) do
+    fn ->
       with {:ok, qr_token} <- fetch_qr_token(token),
            session_attrs = Map.put(device_attrs, :user_id, qr_token.user_id),
            {:ok, session} <- create_mobile_session(session_attrs) do
@@ -182,12 +189,21 @@ defmodule Towerops.MobileSessions do
         |> QRLoginToken.complete_changeset(session.id)
         |> Repo.update!()
 
-        session
+        {:ok, session}
       else
-        {:error, :invalid_token} -> Repo.rollback(:invalid_token)
-        {:error, changeset} -> Repo.rollback(changeset)
+        {:error, :invalid_token} -> {:error, :invalid_token}
+        {:error, changeset} -> {:error, changeset}
       end
-    end)
+    end
+  end
+
+  # Helper function to unwrap nested transaction results
+  defp unwrap_transaction_result(result) do
+    case result do
+      {:ok, {:ok, value}} -> {:ok, value}
+      {:ok, {:error, reason}} -> {:error, reason}
+      {:error, reason} -> {:error, reason}
+    end
   end
 
   defp fetch_qr_token(token) do