fix validation behaviour

lib/mv/accounts/user/validations/email_not_used_by_other_member.ex

@@ -1,26 +1,46 @@
 defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do
   @moduledoc """
   Validates that the user's email is not already used by another member.
-  Allows syncing with linked member (excludes member_id from check).
+  Only validates when:
+  - User is already linked to a member (member_id != nil) AND email is changing
+  - User is being linked to a member (member relationship is changing)
+
+  This allows creating users with the same email as unlinked members.
   """
   use Ash.Resource.Validation
 
   @impl true
   def validate(changeset, _opts, _context) do
-    case Ash.Changeset.fetch_change(changeset, :email) do
+    email_changing? = Ash.Changeset.changing_attribute?(changeset, :email)
-      {:ok, new_email} ->
+    member_changing? = Ash.Changeset.changing_relationship?(changeset, :member)
-        member_id = Ash.Changeset.get_attribute(changeset, :member_id)
-        check_email_uniqueness(new_email, member_id)
 
-      :error ->
+    member_id = Ash.Changeset.get_attribute(changeset, :member_id)
-        :ok
+    is_linked? = not is_nil(member_id)
+
+    # Only validate if:
+    # 1. User is linked AND email is changing
+    # 2. User is being linked/unlinked (member relationship changing)
+    should_validate? = (is_linked? and email_changing?) or member_changing?
+
+    if should_validate? do
+      case Ash.Changeset.fetch_change(changeset, :email) do
+        {:ok, new_email} ->
+          check_email_uniqueness(new_email, member_id)
+
+        :error ->
+          # No email change, get current email
+          current_email = Ash.Changeset.get_attribute(changeset, :email)
+          check_email_uniqueness(current_email, member_id)
+      end
+    else
+      :ok
     end
   end
 
-  defp check_email_uniqueness(new_email, exclude_member_id) do
+  defp check_email_uniqueness(email, exclude_member_id) do
     query =
       Mv.Membership.Member
-      |> Ash.Query.filter(email == ^to_string(new_email))
+      |> Ash.Query.filter(email == ^to_string(email))
       |> maybe_exclude_id(exclude_member_id)
 
     case Ash.read(query) do
@@ -28,7 +48,7 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do
         :ok
 
       {:ok, _} ->
-        {:error, field: :email, message: "is already used by another member", value: new_email}
+        {:error, field: :email, message: "is already used by another member", value: email}
 
       {:error, _} ->
         :ok

lib/mv/membership/member/validations/email_not_used_by_other_user.ex

@@ -1,26 +1,37 @@
 defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do
   @moduledoc """
   Validates that the member's email is not already used by another user.
-  Allows syncing with linked user (excludes linked user from check).
+  Only validates when:
+  - Member is already linked to a user (user != nil) AND email is changing
+  - Member is being linked to a user (user relationship is changing)
+
+  This allows creating members with the same email as unlinked users.
   """
   use Ash.Resource.Validation
 
   @impl true
   def validate(changeset, _opts, _context) do
-    case Ash.Changeset.fetch_change(changeset, :email) do
+    email_changing? = Ash.Changeset.changing_attribute?(changeset, :email)
-      {:ok, new_email} ->
-        linked_user_id = get_linked_user_id(changeset.data)
-        check_email_uniqueness(new_email, linked_user_id)
 
-      :error ->
+    linked_user_id = get_linked_user_id(changeset.data)
-        :ok
+    is_linked? = not is_nil(linked_user_id)
+
+    # Only validate if member is already linked AND email is changing
+    # Do NOT validate when member is being linked (email will be overridden from user)
+    should_validate? = is_linked? and email_changing?
+
+    if should_validate? do
+      new_email = Ash.Changeset.get_attribute(changeset, :email)
+      check_email_uniqueness(new_email, linked_user_id)
+    else
+      :ok
     end
   end
 
-  defp check_email_uniqueness(new_email, exclude_user_id) do
+  defp check_email_uniqueness(email, exclude_user_id) do
     query =
       Mv.Accounts.User
-      |> Ash.Query.filter(email == ^new_email)
+      |> Ash.Query.filter(email == ^email)
       |> maybe_exclude_id(exclude_user_id)
 
     case Ash.read(query) do
@@ -28,7 +39,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do
         :ok
 
       {:ok, _} ->
-        {:error, field: :email, message: "is already used by another user", value: new_email}
+        {:error, field: :email, message: "is already used by another user", value: email}
 
       {:error, _} ->
         :ok

test/accounts/email_uniqueness_test.exs

@@ -4,32 +4,26 @@ defmodule Mv.Accounts.EmailUniquenessTest do
   alias Mv.Accounts
   alias Mv.Membership
 
-  describe "Email uniqueness validation" do
+  describe "Email uniqueness validation - Creation" do
-    test "cannot create member with existing unlinked user email" do
+    test "CAN create member with existing unlinked user email" do
       # Create a user with email
       {:ok, _user} =
         Accounts.create_user(%{
           email: "existing@example.com"
         })
 
-      # Try to create member with same email
+      # Create member with same email - should succeed
-      result =
+      {:ok, member} =
         Membership.create_member(%{
           first_name: "John",
           last_name: "Doe",
           email: "existing@example.com"
         })
 
-      assert {:error, %Ash.Error.Invalid{} = error} = result
+      assert to_string(member.email) == "existing@example.com"
-
-      assert error.errors
-             |> Enum.any?(fn e ->
-               e.field == :email and
-                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
-             end)
     end
 
-    test "cannot create user with existing unlinked member email" do
+    test "CAN create user with existing unlinked member email" do
       # Create a member with email
       {:ok, _member} =
         Membership.create_member(%{
@@ -38,29 +32,25 @@ defmodule Mv.Accounts.EmailUniquenessTest do
           email: "existing@example.com"
         })
 
-      # Try to create user with same email
+      # Create user with same email - should succeed
-      result =
+      {:ok, user} =
         Accounts.create_user(%{
           email: "existing@example.com"
         })
 
-      assert {:error, %Ash.Error.Invalid{} = error} = result
+      assert to_string(user.email) == "existing@example.com"
-
-      assert error.errors
-             |> Enum.any?(fn e ->
-               e.field == :email and
-                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
-             end)
     end
+  end
 
-    test "member email cannot be changed to an existing unlinked user email" do
+  describe "Email uniqueness validation - Updating unlinked entities" do
+    test "unlinked member email CAN be changed to an existing unlinked user email" do
       # Create a user with email
-      {:ok, user} =
+      {:ok, _user} =
         Accounts.create_user(%{
           email: "existing_user@example.com"
         })
 
-      # Create a member with different email
+      # Create an unlinked member with different email
       {:ok, member} =
         Membership.create_member(%{
           first_name: "John",
@@ -68,42 +58,68 @@ defmodule Mv.Accounts.EmailUniquenessTest do
           email: "member@example.com"
         })
 
-      # Try to change member email to existing user email
+      # Change member email to existing user email - should succeed (member is unlinked)
-      result =
+      {:ok, updated_member} =
         Membership.update_member(member, %{
           email: "existing_user@example.com"
         })
 
-      assert {:error, %Ash.Error.Invalid{} = error} = result
+      assert to_string(updated_member.email) == "existing_user@example.com"
-
-      assert error.errors
-             |> Enum.any?(fn e ->
-               e.field == :email and
-                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
-             end)
     end
 
-    test "user email cannot be changed to an existing unlinked member email" do
+    test "unlinked user email CAN be changed to an existing unlinked member email" do
       # Create a member with email
-      {:ok, member} =
+      {:ok, _member} =
         Membership.create_member(%{
           first_name: "John",
           last_name: "Doe",
           email: "existing_member@example.com"
         })
 
-      # Create a user with different email
+      # Create an unlinked user with different email
       {:ok, user} =
         Accounts.create_user(%{
           email: "user@example.com"
         })
 
-      # Try to change user email to existing member email
+      # Change user email to existing member email - should succeed (user is unlinked)
-      result =
+      {:ok, updated_user} =
         Accounts.update_user(user, %{
           email: "existing_member@example.com"
         })
 
+      assert to_string(updated_user.email) == "existing_member@example.com"
+    end
+
+    test "unlinked member email CANNOT be changed to an existing linked user email" do
+      # Create a user and link it to a member - this makes the user "linked"
+      {:ok, user} =
+        Accounts.create_user(%{
+          email: "linked_user@example.com"
+        })
+
+      {:ok, _member_a} =
+        Membership.create_member(%{
+          first_name: "Member",
+          last_name: "A",
+          email: "temp@example.com",
+          user: %{id: user.id}
+        })
+
+      # Create an unlinked member with different email
+      {:ok, member_b} =
+        Membership.create_member(%{
+          first_name: "Member",
+          last_name: "B",
+          email: "member_b@example.com"
+        })
+
+      # Try to change unlinked member's email to linked user's email - should fail
+      result =
+        Membership.update_member(member_b, %{
+          email: "linked_user@example.com"
+        })
+
       assert {:error, %Ash.Error.Invalid{} = error} = result
 
       assert error.errors
@@ -113,6 +129,269 @@ defmodule Mv.Accounts.EmailUniquenessTest do
              end)
     end
 
+    test "unlinked user email CANNOT be changed to an existing linked member email" do
+      # Create a user and link it to a member - this makes the member "linked"
+      {:ok, user_a} =
+        Accounts.create_user(%{
+          email: "user_a@example.com"
+        })
+
+      {:ok, _member_a} =
+        Membership.create_member(%{
+          first_name: "Member",
+          last_name: "A",
+          email: "temp@example.com",
+          user: %{id: user_a.id}
+        })
+
+      # Reload user to get updated member_id and linked member email
+      {:ok, user_a_reloaded} = Ash.get(Mv.Accounts.User, user_a.id)
+      {:ok, user_a_with_member} = Ash.load(user_a_reloaded, :member)
+      linked_member_email = to_string(user_a_with_member.member.email)
+
+      # Create an unlinked user with different email
+      {:ok, user_b} =
+        Accounts.create_user(%{
+          email: "user_b@example.com"
+        })
+
+      # Try to change unlinked user's email to linked member's email - should fail
+      result =
+        Accounts.update_user(user_b, %{
+          email: linked_member_email
+        })
+
+      assert {:error, %Ash.Error.Invalid{} = error} = result
+
+      assert error.errors
+             |> Enum.any?(fn e ->
+               e.field == :email and
+                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
+             end)
+    end
+  end
+
+  describe "Email uniqueness validation - Creating with linked emails" do
+    test "CANNOT create member with existing linked user email" do
+      # Create a user and link it to a member
+      {:ok, user} =
+        Accounts.create_user(%{
+          email: "linked@example.com"
+        })
+
+      {:ok, _member} =
+        Membership.create_member(%{
+          first_name: "First",
+          last_name: "Member",
+          email: "temp@example.com",
+          user: %{id: user.id}
+        })
+
+      # Try to create a new member with the linked user's email - should fail
+      result =
+        Membership.create_member(%{
+          first_name: "Second",
+          last_name: "Member",
+          email: "linked@example.com"
+        })
+
+      assert {:error, %Ash.Error.Invalid{} = error} = result
+
+      assert error.errors
+             |> Enum.any?(fn e ->
+               e.field == :email and
+                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
+             end)
+    end
+
+    test "CANNOT create user with existing linked member email" do
+      # Create a user and link it to a member
+      {:ok, user} =
+        Accounts.create_user(%{
+          email: "user@example.com"
+        })
+
+      {:ok, _member} =
+        Membership.create_member(%{
+          first_name: "Member",
+          last_name: "One",
+          email: "temp@example.com",
+          user: %{id: user.id}
+        })
+
+      # Reload user to get the linked member's email
+      {:ok, user_reloaded} = Ash.get(Mv.Accounts.User, user.id)
+      {:ok, user_with_member} = Ash.load(user_reloaded, :member)
+      linked_member_email = to_string(user_with_member.member.email)
+
+      # Try to create a new user with the linked member's email - should fail
+      result =
+        Accounts.create_user(%{
+          email: linked_member_email
+        })
+
+      assert {:error, %Ash.Error.Invalid{} = error} = result
+
+      assert error.errors
+             |> Enum.any?(fn e ->
+               e.field == :email and
+                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
+             end)
+    end
+  end
+
+  describe "Email uniqueness validation - Updating linked entities" do
+    test "linked member email CANNOT be changed to an existing user email" do
+      # Create a user with email
+      {:ok, _other_user} =
+        Accounts.create_user(%{
+          email: "other_user@example.com"
+        })
+
+      # Create a user and link it to a member
+      {:ok, user} =
+        Accounts.create_user(%{
+          email: "user@example.com"
+        })
+
+      {:ok, member} =
+        Membership.create_member(%{
+          first_name: "John",
+          last_name: "Doe",
+          email: "temp@example.com",
+          user: %{id: user.id}
+        })
+
+      # Try to change linked member's email to other user's email - should fail
+      result =
+        Membership.update_member(member, %{
+          email: "other_user@example.com"
+        })
+
+      assert {:error, %Ash.Error.Invalid{} = error} = result
+
+      assert error.errors
+             |> Enum.any?(fn e ->
+               e.field == :email and
+                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
+             end)
+    end
+
+    test "linked user email CANNOT be changed to an existing member email" do
+      # Create a member with email
+      {:ok, _other_member} =
+        Membership.create_member(%{
+          first_name: "Jane",
+          last_name: "Doe",
+          email: "other_member@example.com"
+        })
+
+      # Create a user and link it to a member
+      {:ok, user} =
+        Accounts.create_user(%{
+          email: "user@example.com"
+        })
+
+      {:ok, _member} =
+        Membership.create_member(%{
+          first_name: "John",
+          last_name: "Doe",
+          email: "temp@example.com",
+          user: %{id: user.id}
+        })
+
+      # Reload user to get updated member_id
+      {:ok, user_reloaded} = Ash.get(Mv.Accounts.User, user.id)
+
+      # Try to change linked user's email to other member's email - should fail
+      result =
+        Accounts.update_user(user_reloaded, %{
+          email: "other_member@example.com"
+        })
+
+      assert {:error, %Ash.Error.Invalid{} = error} = result
+
+      assert error.errors
+             |> Enum.any?(fn e ->
+               e.field == :email and
+                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
+             end)
+    end
+  end
+
+  describe "Email uniqueness validation - Linking" do
+    test "CANNOT link user to member if user email is already used by another unlinked member" do
+      # Create a member with email
+      {:ok, _other_member} =
+        Membership.create_member(%{
+          first_name: "Jane",
+          last_name: "Doe",
+          email: "duplicate@example.com"
+        })
+
+      # Create a user with same email
+      {:ok, user} =
+        Accounts.create_user(%{
+          email: "duplicate@example.com"
+        })
+
+      # Create a member to link with the user
+      {:ok, member} =
+        Membership.create_member(%{
+          first_name: "John",
+          last_name: "Smith",
+          email: "john@example.com"
+        })
+
+      # Try to link user to member - should fail because user.email is already used by other_member
+      result =
+        Accounts.update_user(user, %{
+          member: %{id: member.id}
+        })
+
+      assert {:error, %Ash.Error.Invalid{} = error} = result
+
+      assert error.errors
+             |> Enum.any?(fn e ->
+               e.field == :email and
+                 (String.contains?(e.message, "already") or String.contains?(e.message, "used"))
+             end)
+    end
+
+    test "CAN link member to user even if member email is used by another user (member email gets overridden)" do
+      # Create a user with email
+      {:ok, _other_user} =
+        Accounts.create_user(%{
+          email: "duplicate@example.com"
+        })
+
+      # Create a member with same email
+      {:ok, member} =
+        Membership.create_member(%{
+          first_name: "John",
+          last_name: "Doe",
+          email: "duplicate@example.com"
+        })
+
+      # Create a user to link with the member
+      {:ok, user} =
+        Accounts.create_user(%{
+          email: "user@example.com"
+        })
+
+      # Link member to user - should succeed because member.email will be overridden
+      {:ok, updated_member} =
+        Membership.update_member(member, %{
+          user: %{id: user.id}
+        })
+
+      # Member email should now be the same as user email
+      {:ok, member_reloaded} = Ash.get(Mv.Membership.Member, updated_member.id)
+      assert to_string(member_reloaded.email) == "user@example.com"
+    end
+  end
+
+  describe "Email syncing" do
     test "member email syncs to linked user email without validation error" do
       # Create a user
       {:ok, user} =
@@ -148,7 +427,7 @@ defmodule Mv.Accounts.EmailUniquenessTest do
 
       # Create a user linked to this member
       # The override change will set member.email = user.email automatically
-      {:ok, user} =
+      {:ok, _user} =
         Accounts.create_user(%{
           email: "user@example.com",
           member: %{id: member.id}