refactor(types): reconcile @specs with their success typings

This commit is contained in:
Moritz 2026-06-02 11:25:03 +02:00
parent 263857ee26
commit fd8e6ac178
17 changed files with 92 additions and 38 deletions

View file

@ -836,7 +836,10 @@ defmodule Mv.Membership do
- `{:ok, rejected_request}` - Rejected JoinRequest - `{:ok, rejected_request}` - Rejected JoinRequest
- `{:error, error}` - Status error or authorization error - `{:error, error}` - Status error or authorization error
""" """
@spec reject_join_request(String.t(), keyword()) :: {:ok, JoinRequest.t()} | {:error, term()} @spec reject_join_request(String.t(), keyword()) ::
{:ok, JoinRequest.t()}
| {:ok, JoinRequest.t(), [Ash.Notifier.Notification.t()]}
| {:error, term()}
def reject_join_request(id, opts \\ []) do def reject_join_request(id, opts \\ []) do
actor = Keyword.get(opts, :actor) actor = Keyword.get(opts, :actor)

View file

@ -43,6 +43,7 @@ defmodule Mv.Authorization.PermissionSets do
pattern matches and map lookups with no database queries or external calls. pattern matches and map lookups with no database queries or external calls.
""" """
@type permission_set_name :: :own_data | :read_only | :normal_user | :admin
@type scope :: :own | :linked | :all @type scope :: :own | :linked | :all
@type action :: :read | :create | :update | :destroy @type action :: :read | :create | :update | :destroy
@ -88,7 +89,7 @@ defmodule Mv.Authorization.PermissionSets do
iex> PermissionSets.all_permission_sets() iex> PermissionSets.all_permission_sets()
[:own_data, :read_only, :normal_user, :admin] [:own_data, :read_only, :normal_user, :admin]
""" """
@spec all_permission_sets() :: [atom()] @spec all_permission_sets() :: [permission_set_name(), ...]
def all_permission_sets do def all_permission_sets do
[:own_data, :read_only, :normal_user, :admin] [:own_data, :read_only, :normal_user, :admin]
end end
@ -107,7 +108,7 @@ defmodule Mv.Authorization.PermissionSets do
iex> PermissionSets.get_permissions(:invalid) iex> PermissionSets.get_permissions(:invalid)
** (ArgumentError) invalid permission set: :invalid. Must be one of: [:own_data, :read_only, :normal_user, :admin] ** (ArgumentError) invalid permission set: :invalid. Must be one of: [:own_data, :read_only, :normal_user, :admin]
""" """
@spec get_permissions(atom()) :: permission_set() @spec get_permissions(permission_set_name()) :: permission_set()
def get_permissions(set) when set not in [:own_data, :read_only, :normal_user, :admin] do def get_permissions(set) when set not in [:own_data, :read_only, :normal_user, :admin] do
raise ArgumentError, raise ArgumentError,

View file

@ -409,7 +409,7 @@ defmodule Mv.Config do
@doc """ @doc """
Returns the OIDC groups claim name (default "groups"). ENV first, then Settings. Returns the OIDC groups claim name (default "groups"). ENV first, then Settings.
""" """
@spec oidc_groups_claim() :: String.t() | nil @spec oidc_groups_claim() :: String.t()
def oidc_groups_claim do def oidc_groups_claim do
case env_or_setting("OIDC_GROUPS_CLAIM", :oidc_groups_claim) do case env_or_setting("OIDC_GROUPS_CLAIM", :oidc_groups_claim) do
nil -> "groups" nil -> "groups"
@ -492,7 +492,7 @@ defmodule Mv.Config do
- ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_PORT` - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_PORT`
- Settings mode: read from Settings only - Settings mode: read from Settings only
""" """
@spec smtp_port() :: non_neg_integer() | nil @spec smtp_port() :: pos_integer() | nil
def smtp_port do def smtp_port do
if smtp_env_mode?() do if smtp_env_mode?() do
parse_smtp_port_env(System.get_env("SMTP_PORT")) parse_smtp_port_env(System.get_env("SMTP_PORT"))
@ -638,9 +638,15 @@ defmodule Mv.Config do
""" """
@spec mail_from_name() :: String.t() @spec mail_from_name() :: String.t()
def mail_from_name do def mail_from_name do
case System.get_env("MAIL_FROM_NAME") do name =
nil -> get_from_settings(:smtp_from_name) || "Mila" case System.get_env("MAIL_FROM_NAME") do
value -> trim_nil(value) || "Mila" nil -> get_from_settings(:smtp_from_name)
value -> trim_nil(value)
end
case name do
nil -> "Mila"
name -> name
end end
end end

View file

@ -225,7 +225,10 @@ defmodule Mv.Helpers.SystemActor do
# This allows configuration via SYSTEM_ACTOR_EMAIL env var # This allows configuration via SYSTEM_ACTOR_EMAIL env var
@spec system_user_email_config() :: String.t() @spec system_user_email_config() :: String.t()
defp system_user_email_config do defp system_user_email_config do
System.get_env("SYSTEM_ACTOR_EMAIL") || "system@mila.local" case System.get_env("SYSTEM_ACTOR_EMAIL") do
nil -> "system@mila.local"
email -> email
end
end end
# Loads the system actor from the database # Loads the system actor from the database
@ -257,7 +260,7 @@ defmodule Mv.Helpers.SystemActor do
end end
# Handles database error when loading system user # Handles database error when loading system user
@spec handle_system_user_error(term()) :: Mv.Accounts.User.t() | no_return() @spec handle_system_user_error({:error, Ash.Error.t()}) :: Mv.Accounts.User.t() | no_return()
defp handle_system_user_error(error) do defp handle_system_user_error(error) do
case load_admin_user_fallback() do case load_admin_user_fallback() do
{:ok, admin_user} -> {:ok, admin_user} ->
@ -393,15 +396,18 @@ defmodule Mv.Helpers.SystemActor do
# 1. Only creates system user with known email # 1. Only creates system user with known email
# 2. Only called during system actor initialization (bootstrap) # 2. Only called during system actor initialization (bootstrap)
# 3. Once created, all subsequent operations use proper authorization # 3. Once created, all subsequent operations use proper authorization
Accounts.create_user!(%{email: system_user_email_config()}, user =
upsert?: true, Accounts.create_user!(%{email: system_user_email_config()},
upsert_identity: :unique_email, upsert?: true,
authorize?: false upsert_identity: :unique_email,
) authorize?: false
|> Ash.Changeset.for_update(:update_internal, %{}) )
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) |> Ash.Changeset.for_update(:update_internal, %{})
|> Ash.update!(authorize?: false) |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|> Ash.load!(:role, domain: Mv.Accounts, authorize?: false) |> Ash.update!(authorize?: false)
|> Ash.load!(:role, domain: Mv.Accounts, authorize?: false)
%Accounts.User{} = user
end end
# Finds a user by email address # Finds a user by email address

View file

@ -100,7 +100,8 @@ defmodule Mv.Membership.Import.CsvParser do
|> String.replace("\r", "\n") |> String.replace("\r", "\n")
end end
@spec get_parser(String.t()) :: module() @spec get_parser(String.t()) ::
Mv.Membership.Import.CsvParserSemicolon | Mv.Membership.Import.CsvParserComma
defp get_parser(";"), do: Mv.Membership.Import.CsvParserSemicolon defp get_parser(";"), do: Mv.Membership.Import.CsvParserSemicolon
defp get_parser(","), do: Mv.Membership.Import.CsvParserComma defp get_parser(","), do: Mv.Membership.Import.CsvParserComma
defp get_parser(_), do: Mv.Membership.Import.CsvParserSemicolon defp get_parser(_), do: Mv.Membership.Import.CsvParserSemicolon
@ -116,7 +117,10 @@ defmodule Mv.Membership.Import.CsvParser do
if semicolon_score >= comma_score, do: ";", else: "," if semicolon_score >= comma_score, do: ";", else: ","
end end
@spec header_field_count(module(), binary()) :: non_neg_integer() @spec header_field_count(
Mv.Membership.Import.CsvParserSemicolon | Mv.Membership.Import.CsvParserComma,
binary()
) :: non_neg_integer()
defp header_field_count(parser, header_record) do defp header_field_count(parser, header_record) do
case parse_single_record(parser, header_record, nil) do case parse_single_record(parser, header_record, nil) do
{:ok, fields} -> Enum.count(fields, &(String.trim(&1) != "")) {:ok, fields} -> Enum.count(fields, &(String.trim(&1) != ""))

View file

@ -16,6 +16,21 @@ defmodule Mv.Membership.MemberExport do
alias MvWeb.MemberLive.Index alias MvWeb.MemberLive.Index
alias MvWeb.MemberLive.Index.MembershipFeeStatus alias MvWeb.MemberLive.Index.MembershipFeeStatus
@typedoc "Validated export parameters produced by `parse_params/1`."
@type parsed_params :: %{
selected_ids: [String.t()],
member_fields: [String.t()],
selectable_member_fields: [String.t()],
computed_fields: [String.t()],
custom_field_ids: [String.t()],
query: String.t() | nil,
sort_field: String.t() | nil,
sort_order: String.t() | nil,
show_current_cycle: boolean(),
cycle_status_filter: :paid | :unpaid | nil,
boolean_filters: %{optional(String.t()) => boolean()}
}
@member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++ @member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++
["membership_fee_type", "membership_fee_status", "groups"] ["membership_fee_type", "membership_fee_status", "groups"]
@computed_export_fields ["membership_fee_status"] @computed_export_fields ["membership_fee_status"]
@ -305,7 +320,7 @@ defmodule Mv.Membership.MemberExport do
:computed_fields, :custom_field_ids, :query, :sort_field, :sort_order, :computed_fields, :custom_field_ids, :query, :sort_field, :sort_order,
:show_current_cycle, :cycle_status_filter, :boolean_filters. :show_current_cycle, :cycle_status_filter, :boolean_filters.
""" """
@spec parse_params(map()) :: map() @spec parse_params(map()) :: parsed_params()
def parse_params(params) do def parse_params(params) do
# DB fields come from "member_fields" # DB fields come from "member_fields"
raw_member_fields = extract_list(params, "member_fields") raw_member_fields = extract_list(params, "member_fields")

View file

@ -21,7 +21,7 @@ defmodule Mv.Membership.MembersCSV do
Returns iodata suitable for `IO.iodata_to_binary/1` or sending as response body. Returns iodata suitable for `IO.iodata_to_binary/1` or sending as response body.
RFC 4180 escaping and formula-injection safe_cell are applied. RFC 4180 escaping and formula-injection safe_cell are applied.
""" """
@spec export([struct() | map()], [map()]) :: iodata() @spec export([struct() | map()], [map()]) :: [iodata()] | Enumerable.t()
def export(members, columns) when is_list(members) do def export(members, columns) when is_list(members) do
header = build_header(columns) header = build_header(columns)
rows = Enum.map(members, fn member -> build_row(member, columns) end) rows = Enum.map(members, fn member -> build_row(member, columns) end)

View file

@ -58,7 +58,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do
{:ok, %{success: 45, failed: 0, total: 45}} {:ok, %{success: 45, failed: 0, total: 45}}
""" """
@spec run() :: {:ok, map()} | {:error, term()} @spec run() :: {:ok, CycleGenerator.results_summary()} | {:error, Ash.Error.t()}
def run do def run do
Logger.info("Starting membership fee cycle generation job") Logger.info("Starting membership fee cycle generation job")
start_time = System.monotonic_time(:millisecond) start_time = System.monotonic_time(:millisecond)
@ -98,7 +98,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do
Mv.MembershipFees.CycleGenerationJob.run(batch_size: 5) Mv.MembershipFees.CycleGenerationJob.run(batch_size: 5)
""" """
@spec run(keyword()) :: {:ok, map()} | {:error, term()} @spec run(keyword()) :: {:ok, CycleGenerator.results_summary()} | {:error, Ash.Error.t()}
def run(opts) when is_list(opts) do def run(opts) when is_list(opts) do
Logger.info("Starting membership fee cycle generation job with opts: #{inspect(opts)}") Logger.info("Starting membership fee cycle generation job with opts: #{inspect(opts)}")
start_time = System.monotonic_time(:millisecond) start_time = System.monotonic_time(:millisecond)
@ -135,7 +135,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do
- `{:error, reason}` - Error with reason - `{:error, reason}` - Error with reason
""" """
@spec pending_members_count() :: {:ok, non_neg_integer()} | {:error, term()} @spec pending_members_count() :: {:ok, non_neg_integer()} | {:error, Ash.Error.t()}
def pending_members_count do def pending_members_count do
today = Date.utc_today() today = Date.utc_today()
@ -166,7 +166,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do
- `{:error, reason}` - Error with reason - `{:error, reason}` - Error with reason
""" """
@spec run_for_member(String.t()) :: {:ok, [map()]} | {:error, term()} @spec run_for_member(String.t()) :: CycleGenerator.generate_result()
def run_for_member(member_id) when is_binary(member_id) do def run_for_member(member_id) when is_binary(member_id) do
Logger.info("Generating cycles for member #{member_id}") Logger.info("Generating cycles for member #{member_id}")
CycleGenerator.generate_cycles_for_member(member_id) CycleGenerator.generate_cycles_for_member(member_id)

View file

@ -1,4 +1,11 @@
defmodule Mv.MembershipFees.CycleGenerator do defmodule Mv.MembershipFees.CycleGenerator do
@typedoc "Aggregate counts returned by a batch cycle-generation run."
@type results_summary :: %{
success: non_neg_integer(),
failed: non_neg_integer(),
total: non_neg_integer()
}
@moduledoc """ @moduledoc """
Module for generating membership fee cycles for members. Module for generating membership fee cycles for members.
@ -159,7 +166,8 @@ defmodule Mv.MembershipFees.CycleGenerator do
- `{:error, reason}` - Error with reason - `{:error, reason}` - Error with reason
""" """
@spec generate_cycles_for_all_members(keyword()) :: {:ok, map()} | {:error, term()} @spec generate_cycles_for_all_members(keyword()) ::
{:ok, results_summary()} | {:error, Ash.Error.t()}
def generate_cycles_for_all_members(opts \\ []) do def generate_cycles_for_all_members(opts \\ []) do
today = Keyword.get(opts, :today, Date.utc_today()) today = Keyword.get(opts, :today, Date.utc_today())
batch_size = Keyword.get(opts, :batch_size, 10) batch_size = Keyword.get(opts, :batch_size, 10)

View file

@ -8,6 +8,12 @@ defmodule Mv.Vereinfacht.Client do
""" """
require Logger require Logger
@typedoc "Error reasons returned by Vereinfacht API calls."
@type error_reason ::
:not_configured
| {:request_failed, map()}
| {:http, non_neg_integer(), :html_response | binary()}
@content_type "application/vnd.api+json" @content_type "application/vnd.api+json"
@doc """ @doc """
@ -31,7 +37,7 @@ defmodule Mv.Vereinfacht.Client do
{:error, :not_configured} {:error, :not_configured}
""" """
@spec test_connection(String.t() | nil, String.t() | nil, String.t() | nil) :: @spec test_connection(String.t() | nil, String.t() | nil, String.t() | nil) ::
{:ok, :connected} | {:error, term()} {:ok, :connected} | {:error, error_reason()}
def test_connection(api_url, api_key, club_id) do def test_connection(api_url, api_key, club_id) do
if blank?(api_url) or blank?(api_key) or blank?(club_id) do if blank?(api_url) or blank?(api_key) or blank?(club_id) do
{:error, :not_configured} {:error, :not_configured}
@ -230,7 +236,7 @@ defmodule Mv.Vereinfacht.Client do
Returns the full response body (decoded JSON) for debugging/display. Returns the full response body (decoded JSON) for debugging/display.
""" """
@spec get_contact(String.t()) :: {:ok, map()} | {:error, term()} @spec get_contact(String.t()) :: {:ok, map()} | {:error, error_reason()}
def get_contact(contact_id) when is_binary(contact_id) do def get_contact(contact_id) when is_binary(contact_id) do
fetch_contact(contact_id, []) fetch_contact(contact_id, [])
end end

View file

@ -26,7 +26,7 @@ defmodule Mv.Vereinfacht do
- `{:error, {:http, status, message}}` API returned an error (e.g. 401, 403) - `{:error, {:http, status, message}}` API returned an error (e.g. 401, 403)
- `{:error, {:request_failed, reason}}` network/transport error - `{:error, {:request_failed, reason}}` network/transport error
""" """
@spec test_connection() :: {:ok, :connected} | {:error, term()} @spec test_connection() :: {:ok, :connected} | {:error, Mv.Vereinfacht.Client.error_reason()}
def test_connection do def test_connection do
Client.test_connection( Client.test_connection(
Mv.Config.vereinfacht_api_url(), Mv.Config.vereinfacht_api_url(),

View file

@ -190,7 +190,7 @@ defmodule MvWeb.MemberLive.Index.FieldVisibility do
These fields are not in the database; they must not be used for Ash query These fields are not in the database; they must not be used for Ash query
select/sort. Use this to filter sort options and validate sort_field. select/sort. Use this to filter sort options and validate sort_field.
""" """
@spec computed_member_fields() :: [atom()] @spec computed_member_fields() :: [:membership_fee_status | :membership_fee_type | :groups, ...]
def computed_member_fields, do: @pseudo_member_fields def computed_member_fields, do: @pseudo_member_fields
@doc """ @doc """

View file

@ -341,7 +341,7 @@ defmodule MvWeb.MembershipFeeTypeLive.Form do
end end
end end
@spec notify_parent(any()) :: any() @spec notify_parent(any()) :: {module(), any()}
defp notify_parent(msg), do: send(self(), {__MODULE__, msg}) defp notify_parent(msg), do: send(self(), {__MODULE__, msg})
@spec assign_form(Phoenix.LiveView.Socket.t()) :: Phoenix.LiveView.Socket.t() @spec assign_form(Phoenix.LiveView.Socket.t()) :: Phoenix.LiveView.Socket.t()

View file

@ -186,7 +186,7 @@ defmodule MvWeb.RoleLive.Form do
end end
end end
@spec notify_parent(any()) :: any() @spec notify_parent(any()) :: {module(), any()}
defp notify_parent(msg), do: send(self(), {__MODULE__, msg}) defp notify_parent(msg), do: send(self(), {__MODULE__, msg})
@spec assign_form(Phoenix.LiveView.Socket.t()) :: Phoenix.LiveView.Socket.t() @spec assign_form(Phoenix.LiveView.Socket.t()) :: Phoenix.LiveView.Socket.t()

View file

@ -775,7 +775,7 @@ defmodule MvWeb.UserLive.Form do
)} )}
end end
@spec notify_parent(any()) :: any() @spec notify_parent(any()) :: {module(), any()}
defp notify_parent(msg), do: send(self(), {__MODULE__, msg}) defp notify_parent(msg), do: send(self(), {__MODULE__, msg})
# Helper to ignore keyboard events when dropdown is closed # Helper to ignore keyboard events when dropdown is closed
@ -913,7 +913,7 @@ defmodule MvWeb.UserLive.Form do
MemberResource.filter_by_email_match(members, user_email_str) MemberResource.filter_by_email_match(members, user_email_str)
end end
@spec load_roles(any()) :: [Mv.Authorization.Role.t()] @spec load_roles(any()) :: [Mv.Authorization.Role.t()] | Ash.Page.page()
defp load_roles(actor) do defp load_roles(actor) do
case Authorization.list_roles(actor: actor) do case Authorization.list_roles(actor: actor) do
{:ok, roles} -> roles {:ok, roles} -> roles

View file

@ -145,7 +145,10 @@ defmodule MvWeb.LiveHelpers do
end end
""" """
@spec submit_form(AshPhoenix.Form.t(), map(), Mv.Accounts.User.t() | nil) :: @spec submit_form(AshPhoenix.Form.t(), map(), Mv.Accounts.User.t() | nil) ::
{:ok, Ash.Resource.t()} | {:error, AshPhoenix.Form.t()} {:ok, Ash.Resource.record() | nil | [Ash.Notifier.Notification.t()]}
| {:ok, Ash.Resource.record(), [Ash.Notifier.Notification.t()]}
| :ok
| {:error, AshPhoenix.Form.t()}
def submit_form(form, params, actor) do def submit_form(form, params, actor) do
AshPhoenix.Form.submit(form, params: params, action_opts: ash_actor_opts(actor)) AshPhoenix.Form.submit(form, params: params, action_opts: ash_actor_opts(actor))
end end

View file

@ -12,7 +12,9 @@ defmodule MvWeb.Translations.FieldTypes do
""" """
use Gettext, backend: MvWeb.Gettext use Gettext, backend: MvWeb.Gettext
@spec label(atom()) :: String.t() @type field_type :: :string | :integer | :boolean | :date | :email
@spec label(field_type()) :: String.t()
def label(:string), do: gettext("Text") def label(:string), do: gettext("Text")
def label(:integer), do: gettext("Number") def label(:integer), do: gettext("Number")
def label(:boolean), do: gettext("Yes/No-Selection") def label(:boolean), do: gettext("Yes/No-Selection")