create logical link between users and members closes #164 #172

Merged
moritz merged 14 commits from feature/user-member-relation into main 2025-10-16 16:29:49 +02:00
4 changed files with 296 additions and 1 deletions
Showing only changes of commit 72a8415cb3 - Show all commits

View file

@ -60,15 +60,54 @@ defmodule Mv.Accounts.User do
end end
actions do actions do
defaults [:read, :create, :destroy, :update] defaults [:read, :create, :destroy]
moritz marked this conversation as resolved Outdated

Should we remove the :create here as well, in favor of the :create_user action?

Should we remove the `:create` here as well, in favor of the `:create_user` action?

Used by AshAdmin's generated "Create" UI and by generic AshPhoenix helpers that assume a default create action.

Used by AshAdmin's generated "Create" UI and by generic AshPhoenix helpers that assume a default create action.
update :update do
moritz marked this conversation as resolved Outdated

Are we still using this? If so, can you add a comment explaining the difference to the :update_user action?

Are we still using this? If so, can you add a comment explaining the difference to the `:update_user` action?

Selected by AshAdmin's generated "Edit" UI and generic AshPhoenix helpers that assume a default update action.

Selected by AshAdmin's generated "Edit" UI and generic AshPhoenix helpers that assume a default update action.
primary? true
require_atomic? false
end
create :create_user do create :create_user do
# Only accept email directly - member_id is NOT in accept list
# This prevents direct foreign key manipulation, forcing use of manage_relationship
accept [:email] accept [:email]
# Allow member to be passed as argument for relationship management
argument :member, :map, allow_nil?: true
upsert? true upsert? true
# Manage the member relationship during user creation
change manage_relationship(:member, :member,
# Look up existing member and relate to it
on_lookup: :relate,
# Error if member doesn't exist in database
on_no_match: :error,
# If member already linked to this user, ignore (shouldn't happen in create)
on_match: :ignore,
# If no member provided, that's fine (optional relationship)
on_missing: :ignore
)
end end
update :update_user do update :update_user do
# Only accept email directly - member_id is NOT in accept list
# This prevents direct foreign key manipulation, forcing use of manage_relationship
accept [:email] accept [:email]
# Allow member to be passed as argument for relationship management
argument :member, :map, allow_nil?: true
# Required because custom validation function cannot be done atomically
require_atomic? false
# Manage the member relationship during user update
change manage_relationship(:member, :member,
# Look up existing member and relate to it
on_lookup: :relate,
# Error if member doesn't exist in database
on_no_match: :error,
# If same member provided, that's fine (allows updates with same member)
on_match: :ignore,
# If no member provided, remove existing relationship (allows member removal)
on_missing: :unrelate
)
end end
# Admin action for direct password changes in admin panel # Admin action for direct password changes in admin panel
@ -76,6 +115,7 @@ defmodule Mv.Accounts.User do
update :admin_set_password do update :admin_set_password do
accept [:email] accept [:email]
argument :password, :string, allow_nil?: false, sensitive?: true argument :password, :string, allow_nil?: false, sensitive?: true
require_atomic? false
moritz marked this conversation as resolved Outdated

Should we add this comment here as well?

      # Required because custom validation function cannot be done atomically 
Should we add this comment here as well? ```elixir # Required because custom validation function cannot be done atomically ```
# Set the strategy context that HashPasswordChange expects # Set the strategy context that HashPasswordChange expects
change set_context(%{strategy_name: :password}) change set_context(%{strategy_name: :password})
@ -125,6 +165,28 @@ defmodule Mv.Accounts.User do
validate string_length(:password, min: 8) do validate string_length(:password, min: 8) do
where action_is([:register_with_password, :admin_set_password]) where action_is([:register_with_password, :admin_set_password])
end end
# Prevent overwriting existing member relationship
# This validation ensures race condition safety by requiring explicit two-step process:
# 1. Remove existing member (set member to nil)
# 2. Add new member
# This prevents accidental overwrites when multiple admins work simultaneously
validate fn changeset, _context ->
member_arg = Ash.Changeset.get_argument(changeset, :member)
current_member_id = changeset.data.member_id
# Only trigger if:
# - member argument is provided AND has an ID
# - user currently has a member
# - the new member ID is different from current member ID
if member_arg && member_arg[:id] && current_member_id &&
member_arg[:id] != current_member_id do
{:error,
field: :member, message: "User already has a member. Remove existing member first."}
moritz marked this conversation as resolved Outdated

For translation reasons, gettext should be used for all those error messages

For translation reasons, gettext should be used for all those error messages

These messages are translated by ash. Only the translation text was missing.

These messages are translated by ash. Only the translation text was missing.
else
:ok
end
end
end end
def validate_oidc_id_present(changeset, _context) do def validate_oidc_id_present(changeset, _context) do
@ -146,12 +208,16 @@ defmodule Mv.Accounts.User do
end end
relationships do relationships do
# 1:1 relationship - User can optionally belong to one Member
# This automatically creates a `member_id` attribute in the User table
# The relationship is optional (allow_nil? true by default)
belongs_to :member, Mv.Membership.Member belongs_to :member, Mv.Membership.Member
end end
identities do identities do
identity :unique_email, [:email] identity :unique_email, [:email]
identity :unique_oidc_id, [:oidc_id] identity :unique_oidc_id, [:oidc_id]
identity :unique_member, [:member_id]
end end
# You can customize this if you wish, but this is a safe default that # You can customize this if you wish, but this is a safe default that

View file

@ -13,7 +13,11 @@ defmodule Mv.Membership.Member do
create :create_member do create :create_member do
primary? true primary? true
# Properties can be created along with member
argument :properties, {:array, :map} argument :properties, {:array, :map}
# Allow user to be passed as argument for relationship management
# user_id is NOT in accept list to prevent direct foreign key manipulation
argument :user, :map, allow_nil?: true
accept [ accept [
:first_name, :first_name,
@ -32,12 +36,29 @@ defmodule Mv.Membership.Member do
] ]
change manage_relationship(:properties, type: :create) change manage_relationship(:properties, type: :create)
# Manage the user relationship during member creation
change manage_relationship(:user, :user,
# Look up existing user and relate to it
on_lookup: :relate,
# Error if user doesn't exist in database
on_no_match: :error,
# Error if user is already linked to another member (prevents "stealing")
on_match: :error,
# If no user provided, that's fine (optional relationship)
on_missing: :ignore
)
end end
update :update_member do update :update_member do
primary? true primary? true
# Required because custom validation function cannot be done atomically
require_atomic? false require_atomic? false
# Properties can be updated or created along with member
argument :properties, {:array, :map} argument :properties, {:array, :map}
# Allow user to be passed as argument for relationship management
# user_id is NOT in accept list to prevent direct foreign key manipulation
argument :user, :map, allow_nil?: true
accept [ accept [
:first_name, :first_name,
@ -56,6 +77,18 @@ defmodule Mv.Membership.Member do
] ]
change manage_relationship(:properties, on_match: :update, on_no_match: :create) change manage_relationship(:properties, on_match: :update, on_no_match: :create)
# Manage the user relationship during member update
change manage_relationship(:user, :user,
# Look up existing user and relate to it
on_lookup: :relate,
# Error if user doesn't exist in database
on_no_match: :error,
# Error if user is already linked to another member (prevents "stealing")
on_match: :error,
# If no user provided, remove existing relationship (allows user removal)
on_missing: :unrelate
)
end end
end end
@ -67,6 +100,40 @@ defmodule Mv.Membership.Member do
validate present(:last_name) validate present(:last_name)
validate present(:email) validate present(:email)
# Prevent linking to a user that already has a member
# This validation prevents "stealing" users from other members by checking
# if the target user is already linked to a different member
# This is necessary because manage_relationship's on_match: :error only checks
# if the user is already linked to THIS specific member, not ANY member
validate fn changeset, _context ->
user_arg = Ash.Changeset.get_argument(changeset, :user)
if user_arg && user_arg[:id] do
user_id = user_arg[:id]
current_member_id = changeset.data.id
# Check the current state of the user in the database
case Ash.get(Mv.Accounts.User, user_id) do
# User is free to be linked
{:ok, %{member_id: nil}} ->
:ok
# User already linked to this member (update scenario)
{:ok, %{member_id: ^current_member_id}} ->
:ok
{:ok, %{member_id: _other_member_id}} ->
# User is linked to a different member - prevent "stealing"
{:error, field: :user, message: "User is already linked to another member"}
{:error, _} ->
{:error, field: :user, message: "User not found"}
end
else
:ok
end
end
# Birth date not in the future # Birth date not in the future
validate compare(:birth_date, less_than_or_equal_to: &Date.utc_today/0), validate compare(:birth_date, less_than_or_equal_to: &Date.utc_today/0),
where: [present(:birth_date)], where: [present(:birth_date)],
@ -175,5 +242,9 @@ defmodule Mv.Membership.Member do
relationships do relationships do
has_many :properties, Mv.Membership.Property has_many :properties, Mv.Membership.Property
# 1:1 relationship - Member can optionally have one User
# This references the User's member_id attribute
# The relationship is optional (allow_nil? true by default)
has_one :user, Mv.Accounts.User
end end
end end

View file

@ -0,0 +1,17 @@
defmodule Mv.Repo.Migrations.MemberRelation do
@moduledoc """
Updates resources based on their most recent snapshots.
This file was autogenerated with `mix ash_postgres.generate_migrations`
"""
use Ecto.Migration
def up do
create unique_index(:users, [:member_id], name: "users_unique_member_index")
end
def down do
drop_if_exists unique_index(:users, [:member_id], name: "users_unique_member_index")
end
end

View file

@ -0,0 +1,141 @@
{
"attributes": [
{
"allow_nil?": false,
"default": "fragment(\"gen_random_uuid()\")",
"generated?": false,
"precision": null,
"primary_key?": true,
"references": null,
"scale": null,
"size": null,
"source": "id",
"type": "uuid"
},
{
"allow_nil?": false,
"default": "nil",
"generated?": false,
"precision": null,
"primary_key?": false,
"references": null,
"scale": null,
"size": null,
"source": "email",
"type": "citext"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"precision": null,
"primary_key?": false,
"references": null,
"scale": null,
"size": null,
"source": "hashed_password",
"type": "text"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"precision": null,
"primary_key?": false,
"references": null,
"scale": null,
"size": null,
"source": "oidc_id",
"type": "text"
},
{
"allow_nil?": true,
"default": "nil",
"generated?": false,
"precision": null,
"primary_key?": false,
"references": {
"deferrable": false,
"destination_attribute": "id",
"destination_attribute_default": null,
"destination_attribute_generated": null,
"index?": false,
"match_type": null,
"match_with": null,
"multitenancy": {
"attribute": null,
"global": null,
"strategy": null
},
"name": "users_member_id_fkey",
"on_delete": null,
"on_update": null,
"primary_key?": true,
"schema": "public",
"table": "members"
},
"scale": null,
"size": null,
"source": "member_id",
"type": "uuid"
}
],
"base_filter": null,
"check_constraints": [],
"custom_indexes": [],
"custom_statements": [],
"has_create_action": true,
"hash": "FDEBD4840449609DDA8B50D6741C2EEDE9D81DFBC1E26D4BC77DBD9B5A8EA4DC",
"identities": [
{
"all_tenants?": false,
"base_filter": null,
"index_name": "users_unique_email_index",
"keys": [
{
"type": "atom",
"value": "email"
}
],
"name": "unique_email",
"nils_distinct?": true,
"where": null
},
{
"all_tenants?": false,
"base_filter": null,
"index_name": "users_unique_oidc_id_index",
"keys": [
{
"type": "atom",
"value": "oidc_id"
}
],
"name": "unique_oidc_id",
"nils_distinct?": true,
"where": null
},
{
"all_tenants?": false,
"base_filter": null,
"index_name": "users_unique_member_index",
"keys": [
{
"type": "atom",
"value": "member_id"
}
],
"name": "unique_member",
"nils_distinct?": true,
"where": null
}
],
"multitenancy": {
"attribute": null,
"global": null,
"strategy": null
},
"repo": "Elixir.Mv.Repo",
"schema": null,
"table": "users"
}