local-it/mitgliederverwaltung
2
3
Fork 0
Code Issues 51 Pull requests 13 Projects 1 Releases Packages 1 Activity Actions

Default Memberfields closes #74 #48 #49 #50 #81

Merged
carla merged 6 commits from feature/74_memberfields into main 2025-06-25 14:28:07 +02:00
Conversation 4 Commits 6 Files changed 16 +18 -61
2 changed files with 18 additions and 61 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 7f034740b0 - Show all commits

2
Justfile
View file

@ -9,7 +9,7 @@ migrate-database:
reset-database:
mix ash.reset
MIX_ENV=test mix ash.setup
MIX_ENV=test mix ash.reset
carla marked this conversation as resolved Outdated
rafael commented 2025-06-19 17:04:12 +02:00
Outdated
Copy link

Why not use ash.reset here as well?

Why not use `ash.reset` here as well?
moritz commented 2025-06-19 22:36:58 +02:00
Outdated
Copy link

typo that works as well. but i change it

typo that works as well. but i change it
seed-database:
mix run priv/repo/seeds.exs

77
lib/membership/member.ex
View file

@ -9,7 +9,7 @@ defmodule Mv.Membership.Member do
end
actions do
defaults [:read]
defaults [:read, :destroy]
create :create_member do
primary? true
@ -34,11 +34,6 @@ defmodule Mv.Membership.Member do
change manage_relationship(:properties, type: :create)
end
destroy :destroy do
primary? true
change Ash.Resource.Change.Builtins.cascade_destroy(:properties)
end
update :update_member do
primary? true
require_atomic? false
carla marked this conversation as resolved Outdated
rafael commented 2025-06-19 17:51:23 +02:00
Outdated
Copy link

I think this will run using ash's "notifiers", and as such will run outside the transaction used for the destroy itself. This means that we could end up with errors if postgres detects that a property does not have an associated member.

Also, I think the "references" block in the property's postgres configuration should take make this line obsolete because it tells postgres to cascade delete properties when a member is deleted? 🤔

I think this will run using ash's "notifiers", and as such [will run outside the transaction used for the destroy itself](https://hexdocs.pm/ash/notifiers.html#transactions). This means that we could end up with errors if postgres detects that a property does not have an associated member. Also, I think the "references" block in the property's postgres configuration should take make this line obsolete because it tells postgres to cascade delete properties when a member is deleted? 🤔
moritz commented 2025-06-19 22:44:34 +02:00
Outdated
Copy link

I tried different approaches to get the cascade deletion working. I think in the end the postgres reference block has done the job and this is still a leftover. I will try to run it without this change.

I tried different approaches to get the cascade deletion working. I think in the end the postgres reference block has done the job and this is still a leftover. I will try to run it without this change.
carla commented 2025-06-20 07:35:21 +02:00
Outdated
Copy link

Seems that properties are deleted using default :destroy with the reference block

Seems that properties are deleted using default :destroy with the reference block
@ -73,71 +68,33 @@ defmodule Mv.Membership.Member do
validate present(:email)
# Birth date not in the future
validate fn changeset, _ ->
birth_date = Ash.Changeset.get_attribute(changeset, :birth_date)
if birth_date && Date.compare(birth_date, Date.utc_today()) == :gt do
{:error, field: :birth_date, message: "cannot be in the future"}
else
:ok
end
end
validate compare(:birth_date, less_than_or_equal_to: &Date.utc_today/0),
where: [present(:birth_date)],
message: "cannot be in the future"
# Join date not in the future
validate fn changeset, _ ->
join_date = Ash.Changeset.get_attribute(changeset, :join_date)
validate compare(:join_date, less_than_or_equal_to: &Date.utc_today/0),
where: [present(:join_date)],
message: "cannot be in the future"
if join_date && Date.compare(join_date, Date.utc_today()) == :gt do
{:error, field: :join_date, message: "cannot be in the future"}
else
:ok
end
end
# Exit date not before join date
validate fn changeset, _ ->
join_date = Ash.Changeset.get_attribute(changeset, :join_date)
exit_date = Ash.Changeset.get_attribute(changeset, :exit_date)
validate compare(:exit_date, greater_than: :join_date),
where: [present([:join_date, :exit_date])],
message: "cannot be before join date"
if join_date && exit_date && Date.compare(exit_date, join_date) == :lt do
{:error, field: :exit_date, message: "cannot be before join date"}
else
:ok
end
end
# Phone number format (only if set)
validate fn changeset, _ ->
phone = Ash.Changeset.get_attribute(changeset, :phone_number)
validate match(:phone_number, ~r/^\+?[0-9\- ]{6,20}$/),
where: [present(:phone_number)],
message: "is not a valid phone number"
carla marked this conversation as resolved Outdated
rafael commented 2025-06-19 17:28:39 +02:00
Outdated
Copy link

I'm fine with doing this using custom validations as I think "real" code is easier to read than DSLs - but I think if we wanted to, we could achieve this in a more concise way using ash's built-in compare validation, e.g.:

validate compare(:join_date, less_than: &Date.utc_today/0),
  where: [present(:join_date)],
  message: "cannot be in the future"

Not sure if I got the syntax exactly right, but I hope you get what I mean. I think this approach would work for other custom validations in this file as well, e.g. using the built-in match validation for regexes.

One advantage of using the built-in validations is that they are "atomic", meaning they can run in the database directly, which can improve performance.

I'm fine with doing this using custom validations as I think "real" code is easier to read than DSLs - but I think if we wanted to, we could achieve this in a more concise way using ash's [built-in compare validation](https://hexdocs.pm/ash/Ash.Resource.Validation.Builtins.html#compare/2), e.g.: ```elixir validate compare(:join_date, less_than: &Date.utc_today/0), where: [present(:join_date)], message: "cannot be in the future" ``` Not sure if I got the syntax exactly right, but I hope you get what I mean. I think this approach would work for other custom validations in this file as well, e.g. using the built-in `match` validation for regexes. One advantage of using the built-in validations is that they are "atomic", meaning they can run in the database directly, which can improve performance.
moritz commented 2025-06-19 22:45:26 +02:00
Outdated
Copy link

Thank you for the hint, I will apply it and test if it works.

Thank you for the hint, I will apply it and test if it works.
carla commented 2025-06-20 07:44:33 +02:00
Outdated
Copy link

Maybe not from a technical but from a user perspective: Why should it not be possible to add members already, where the membership starts maybe beginning of next month? Should we take the question to the Pilotvereine?

Maybe not from a technical but from a user perspective: Why should it not be possible to add members already, where the membership starts maybe beginning of next month? Should we take the question to the Pilotvereine?
👍 2
if phone && !Regex.match?(~r/^\+?[0-9\- ]{6,20}$/, phone) do
{:error, field: :phone_number, message: "is not a valid phone number"}
else
:ok
end
end
# Postal code format (only if set)
validate fn changeset, _ ->
postal_code = Ash.Changeset.get_attribute(changeset, :postal_code)
validate match(:postal_code, ~r/^\d{5}$/),
where: [present(:postal_code)],
message: "must consist of 5 digits"
if postal_code && !Regex.match?(~r/^\d{5}$/, postal_code) do
{:error, field: :postal_code, message: "must consist of 5 digits"}
else
:ok
end
end
# paid must be boolean if set
validate fn changeset, _ ->
paid = Ash.Changeset.get_attribute(changeset, :paid)
if not is_nil(paid) and not is_boolean(paid) do
{:error, field: :paid, message: "must be true or false"}
else
:ok
end
end
# Email validation with EctoCommons.EmailValidator
validate fn changeset, _ ->
@ -216,6 +173,6 @@ defmodule Mv.Membership.Member do
end
relationships do
has_many :properties, Mv.Membership.Property, destination_attribute: :member_id
has_many :properties, Mv.Membership.Property
end
end