From 8eb05c8a6a409492ac01419f0b191aa0e1094418 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Jan 2026 22:09:22 +0100 Subject: [PATCH] Document System Actor pattern in code guidelines Add section explaining when and how to use system actor for systemic operations. Include examples and distinction between user mode and system mode. --- CODE_GUIDELINES.md | 49 ++++++++++++++++++++++++++- lib/mv/helpers/system_actor.ex | 26 ++++++++++---- test/mv/helpers/system_actor_test.exs | 6 +++- 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index fe2d816..604e2af 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -641,7 +641,54 @@ def card(assigns) do end ``` -### 3.3 Ash Framework +### 3.3 System Actor Pattern + +**When to Use System Actor:** + +Some operations must always run regardless of user permissions. These are **systemic operations** that are mandatory side effects: + +- **Email synchronization** (Member ↔ User) +- **Email uniqueness validation** (data integrity requirement) +- **Cycle generation** (if defined as mandatory side effect) +- **Background jobs** +- **Seeds** + +**Implementation:** + +Use `Mv.Helpers.SystemActor.get_system_actor/0` for all systemic operations: + +```elixir +# Good - Email sync uses system actor +def get_linked_member(user) do + system_actor = SystemActor.get_system_actor() + opts = Helpers.ash_actor_opts(system_actor) + + case Ash.get(Mv.Membership.Member, id, opts) do + {:ok, member} -> member + {:error, _} -> nil + end +end + +# Bad - Using user actor for systemic operation +def get_linked_member(user, actor) do + opts = Helpers.ash_actor_opts(actor) # May fail if user lacks permissions! + # ... +end +``` + +**System Actor Details:** + +- System actor is a user with admin role (email: "system@mila.local") +- Cached in Agent for performance +- Falls back to admin user from seeds if system user doesn't exist +- Should NEVER be used for user-initiated actions (only systemic operations) + +**User Mode vs System Mode:** + +- **User Mode**: User-initiated actions use the actual user actor, policies are enforced +- **System Mode**: Systemic operations use system actor, bypass user permissions + +### 3.4 Ash Framework **Resource Definition Best Practices:** diff --git a/lib/mv/helpers/system_actor.ex b/lib/mv/helpers/system_actor.ex index 64f95f1..dd11690 100644 --- a/lib/mv/helpers/system_actor.ex +++ b/lib/mv/helpers/system_actor.ex @@ -124,7 +124,9 @@ defmodule Mv.Helpers.SystemActor do {:ok, nil} -> # System user doesn't exist - fall back to admin user case load_admin_user_fallback() do - {:ok, admin_user} -> admin_user + {:ok, admin_user} -> + admin_user + {:error, _} -> # In test environment, create a temporary admin user if none exists if Mix.env() == :test do @@ -137,7 +139,9 @@ defmodule Mv.Helpers.SystemActor do {:error, _reason} = error -> # Database error - try fallback case load_admin_user_fallback() do - {:ok, admin_user} -> admin_user + {:ok, admin_user} -> + admin_user + {:error, _} -> # In test environment, create a temporary admin user if none exists if Mix.env() == :test do @@ -166,16 +170,21 @@ defmodule Mv.Helpers.SystemActor do description: "Administrator with full access", permission_set_name: "admin" }) do - {:ok, role} -> role - {:error, %Ash.Error.Invalid{errors: [%{field: :name, message: "has already been taken"}]}} -> + {:ok, role} -> + role + + {:error, + %Ash.Error.Invalid{errors: [%{field: :name, message: "has already been taken"}]}} -> # Role was created by another process - find it case Authorization.list_roles() do {:ok, updated_roles} -> Enum.find(updated_roles, &(&1.permission_set_name == "admin")) || raise "Admin role should exist but was not found" + _ -> raise "Failed to find admin role after creation attempt" end + {:error, error} -> raise "Failed to create admin role: #{inspect(error)}" end @@ -191,16 +200,21 @@ defmodule Mv.Helpers.SystemActor do description: "Administrator with full access", permission_set_name: "admin" }) do - {:ok, role} -> role - {:error, %Ash.Error.Invalid{errors: [%{field: :name, message: "has already been taken"}]}} -> + {:ok, role} -> + role + + {:error, + %Ash.Error.Invalid{errors: [%{field: :name, message: "has already been taken"}]}} -> # Role exists - try to find it case Authorization.list_roles() do {:ok, roles} -> Enum.find(roles, &(&1.permission_set_name == "admin")) || raise "Admin role should exist but was not found" + _ -> raise "Failed to find admin role" end + {:error, error} -> raise "Failed to create admin role: #{inspect(error)}" end diff --git a/test/mv/helpers/system_actor_test.exs b/test/mv/helpers/system_actor_test.exs index 2cf6b7f..c1f6d0e 100644 --- a/test/mv/helpers/system_actor_test.exs +++ b/test/mv/helpers/system_actor_test.exs @@ -79,7 +79,10 @@ defmodule Mv.Helpers.SystemActorTest do |> Ash.load!(:role, domain: Mv.Accounts) _ -> - Accounts.create_user!(%{email: admin_email}, upsert?: true, upsert_identity: :unique_email) + Accounts.create_user!(%{email: admin_email}, + upsert?: true, + upsert_identity: :unique_email + ) |> Ash.Changeset.for_update(:update, %{}) |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) |> Ash.update!() @@ -152,6 +155,7 @@ defmodule Mv.Helpers.SystemActorTest do end admin_email = System.get_env("ADMIN_EMAIL") || "admin@localhost" + case Accounts.User |> Ash.Query.filter(email == ^admin_email) |> Ash.read_one(domain: Mv.Accounts) do