From ce57d046b96d970bfb459164787669dc12bfc6b8 Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 1 Jun 2026 21:46:50 +0200 Subject: [PATCH 01/16] ci(drone): run full test suite on main, tags and promote --- .drone.jsonnet | 169 ++++++++++++++++++++++++++++ .drone.yml | 298 ------------------------------------------------- 2 files changed, 169 insertions(+), 298 deletions(-) create mode 100644 .drone.jsonnet delete mode 100644 .drone.yml diff --git a/.drone.jsonnet b/.drone.jsonnet new file mode 100644 index 0000000..ba8bacd --- /dev/null +++ b/.drone.jsonnet @@ -0,0 +1,169 @@ +local elixir = 'docker.io/library/elixir:1.18.3-otp-27'; +local postgres_image = 'docker.io/library/postgres:18.3'; + +local pg_service = { + name: 'postgres', + image: postgres_image, + environment: { + POSTGRES_USER: 'postgres', + POSTGRES_PASSWORD: 'postgres', + }, +}; + +local cache_volume = { name: 'cache', host: { path: '/tmp/drone_cache' } }; +local cache_mount = [{ name: 'cache', path: '/cache' }]; + +local step_compute_cache = { + name: 'compute cache key', + image: elixir, + commands: [ + "mix_lock_hash=$(sha256sum mix.lock | cut -d ' ' -f 1)", + 'echo "$DRONE_REPO_OWNER/$DRONE_REPO_NAME/$mix_lock_hash" >> .cache_key', + // Print cache key for debugging + 'cat .cache_key', + ], +}; + +local step_restore_cache = { + name: 'restore-cache', + image: 'drillster/drone-volume-cache', + settings: { restore: true, mount: ['./deps', './_build'], ttl: 30 }, + volumes: cache_mount, +}; + +local step_lint = { + name: 'lint', + image: elixir, + commands: [ + 'mix local.hex --force', // Install hex package manager + 'mix deps.get', // Fetch dependencies + 'mix compile --warnings-as-errors', // Check for compilation errors & warnings + 'mix format --check-formatted', // Check formatting + 'mix sobelow --config', // Security checks + 'mix deps.audit --ignore-file .deps_audit_ignore', // Known vulnerabilities + 'mix hex.audit', // Unmaintained dependencies + 'mix credo --strict', // Code quality hints + 'mix gettext.extract --check-up-to-date', // Translations up to date + ], +}; + +local step_wait_postgres = { + name: 'wait_for_postgres', + image: postgres_image, + commands: [ + ||| + for i in {1..20}; do + if pg_isready -h postgres -U postgres; then + exit 0 + else + true + fi + sleep 2 + done + echo "Postgres did not become available, aborting." + exit 1 + |||, + ], +}; + +local step_rebuild_cache = { + name: 'rebuild-cache', + image: 'drillster/drone-volume-cache', + settings: { rebuild: true, mount: ['./deps', './_build'] }, + volumes: cache_mount, +}; + +// test_cmd is the only thing that differs between the fast and full suites. +local test_step(name, test_cmd) = { + name: name, + image: elixir, + environment: { + MIX_ENV: 'test', + TEST_POSTGRES_HOST: 'postgres', + TEST_POSTGRES_PORT: '5432', + }, + commands: ['mix local.hex --force', 'mix deps.get', test_cmd], +}; + +local test_fast = test_step('test-fast', 'mix test --exclude slow --exclude ui --max-cases 2'); +local test_all = test_step('test-all', 'mix test'); + +// A full check pipeline: identical steps, only name + trigger + test step vary. +local check_pipeline(name, trigger, test) = { + kind: 'pipeline', + type: 'docker', + name: name, + services: [pg_service], + trigger: trigger, + steps: [ + step_compute_cache, + step_restore_cache, + step_lint, + step_wait_postgres, + test, + step_rebuild_cache, + ], + volumes: [cache_volume], +}; + +local docker_publish(name, extra_settings, trigger_event, deps) = { + kind: 'pipeline', + type: 'docker', + name: name, + trigger: trigger_event, + steps: [{ + name: 'build-and-publish-container' + (if name == 'build-and-publish' then '-branch' else ''), + image: 'plugins/docker', + settings: { + registry: 'git.local-it.org', + repo: 'git.local-it.org/local-it/mitgliederverwaltung', + username: { from_secret: 'DRONE_REGISTRY_USERNAME' }, + password: { from_secret: 'DRONE_REGISTRY_TOKEN' }, + } + extra_settings, + when: trigger_event, + }], + depends_on: deps, +}; + +[ + check_pipeline('check-fast', { branch: { exclude: ['main'] }, event: ['push'] }, test_fast), + check_pipeline('check-full', { branch: ['main'], event: ['push'] }, test_all), + check_pipeline('check-full-promote', { event: ['promote'], target: ['production'] }, test_all), + check_pipeline('check-full-tag', { event: ['tag'] }, test_all), + + docker_publish( + 'build-and-publish', + { tags: ['latest', '${DRONE_COMMIT_SHA:0:8}'] }, + { branch: ['main'], event: ['push'] }, + ['check-full'], + ), + docker_publish( + 'build-and-release', + { auto_tag: true }, + { event: ['tag'] }, + ['check-full-tag'], + ), + + { + kind: 'pipeline', + type: 'docker', + name: 'renovate', + trigger: { event: ['cron', 'custom'], branch: ['main'] }, + environment: { LOG_LEVEL: 'debug' }, + steps: [{ + name: 'renovate', + image: 'renovate/renovate:43.165', + environment: { + RENOVATE_CONFIG_FILE: 'renovate_backend_config.js', + RENOVATE_TOKEN: { from_secret: 'RENOVATE_TOKEN' }, + GITHUB_COM_TOKEN: { from_secret: 'GITHUB_COM_TOKEN' }, + }, + commands: [ + // https://github.com/renovatebot/renovate/discussions/15049 + 'unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL', + 'renovate-config-validator', + 'renovate', + ], + }], + }, +] diff --git a/.drone.yml b/.drone.yml deleted file mode 100644 index b0e1d3d..0000000 --- a/.drone.yml +++ /dev/null @@ -1,298 +0,0 @@ -kind: pipeline -type: docker -name: check-fast - -services: - - name: postgres - image: docker.io/library/postgres:18.3 - environment: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: postgres - -trigger: - event: - - push - -steps: - - name: compute cache key - image: docker.io/library/elixir:1.18.3-otp-27 - commands: - - mix_lock_hash=$(sha256sum mix.lock | cut -d ' ' -f 1) - - echo "$DRONE_REPO_OWNER/$DRONE_REPO_NAME/$mix_lock_hash" >> .cache_key - # Print cache key for debugging - - cat .cache_key - - - name: restore-cache - image: drillster/drone-volume-cache - settings: - restore: true - mount: - - ./deps - - ./_build - ttl: 30 - volumes: - - name: cache - path: /cache - - - name: lint - image: docker.io/library/elixir:1.18.3-otp-27 - commands: - # Install hex package manager - - mix local.hex --force - # Fetch dependencies - - mix deps.get - # Check for compilation errors & warnings - - mix compile --warnings-as-errors - # Check formatting - - mix format --check-formatted - # Security checks - - mix sobelow --config - # Check dependencies for known vulnerabilities - - mix deps.audit --ignore-file .deps_audit_ignore - # Check for dependencies that are not maintained anymore - - mix hex.audit - # Provide hints for improving code quality - - mix credo --strict - # Check that translations are up to date - - mix gettext.extract --check-up-to-date - - - name: wait_for_postgres - image: docker.io/library/postgres:18.3 - commands: - # Wait for postgres to become available - - | - for i in {1..20}; do - if pg_isready -h postgres -U postgres; then - exit 0 - else - true - fi - sleep 2 - done - echo "Postgres did not become available, aborting." - exit 1 - - - name: test-fast - image: docker.io/library/elixir:1.18.3-otp-27 - environment: - MIX_ENV: test - TEST_POSTGRES_HOST: postgres - TEST_POSTGRES_PORT: 5432 - commands: - # Install hex package manager - - mix local.hex --force - # Fetch dependencies - - mix deps.get - # Run fast tests (excludes slow/performance and UI tests) - - mix test --exclude slow --exclude ui --max-cases 2 - - - name: rebuild-cache - image: drillster/drone-volume-cache - settings: - rebuild: true - mount: - - ./deps - - ./_build - volumes: - - name: cache - path: /cache - -volumes: - - name: cache - host: - path: /tmp/drone_cache - ---- -kind: pipeline -type: docker -name: check-full - -services: - - name: postgres - image: docker.io/library/postgres:18.3 - environment: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: postgres - -trigger: - event: - - promote - target: - - production - -steps: - - name: compute cache key - image: docker.io/library/elixir:1.18.3-otp-27 - commands: - - mix_lock_hash=$(sha256sum mix.lock | cut -d ' ' -f 1) - - echo "$DRONE_REPO_OWNER/$DRONE_REPO_NAME/$mix_lock_hash" >> .cache_key - # Print cache key for debugging - - cat .cache_key - - - name: restore-cache - image: drillster/drone-volume-cache - settings: - restore: true - mount: - - ./deps - - ./_build - ttl: 30 - volumes: - - name: cache - path: /cache - - - name: lint - image: docker.io/library/elixir:1.18.3-otp-27 - commands: - # Install hex package manager - - mix local.hex --force - # Fetch dependencies - - mix deps.get - # Check for compilation errors & warnings - - mix compile --warnings-as-errors - # Check formatting - - mix format --check-formatted - # Security checks - - mix sobelow --config - # Check dependencies for known vulnerabilities - - mix deps.audit --ignore-file .deps_audit_ignore - # Check for dependencies that are not maintained anymore - - mix hex.audit - # Provide hints for improving code quality - - mix credo --strict - # Check that translations are up to date - - mix gettext.extract --check-up-to-date - - - name: wait_for_postgres - image: docker.io/library/postgres:18.3 - commands: - # Wait for postgres to become available - - | - for i in {1..20}; do - if pg_isready -h postgres -U postgres; then - exit 0 - else - true - fi - sleep 2 - done - echo "Postgres did not become available, aborting." - exit 1 - - - name: test-all - image: docker.io/library/elixir:1.18.3-otp-27 - environment: - MIX_ENV: test - TEST_POSTGRES_HOST: postgres - TEST_POSTGRES_PORT: 5432 - commands: - # Install hex package manager - - mix local.hex --force - # Fetch dependencies - - mix deps.get - # Run all tests (including slow/performance and UI tests) - - mix test - - - name: rebuild-cache - image: drillster/drone-volume-cache - settings: - rebuild: true - mount: - - ./deps - - ./_build - volumes: - - name: cache - path: /cache - -volumes: - - name: cache - host: - path: /tmp/drone_cache - ---- -kind: pipeline -type: docker -name: build-and-publish - -trigger: - branch: - - main - event: - - push - -steps: - - name: build-and-publish-container-branch - image: plugins/docker - settings: - registry: git.local-it.org - repo: git.local-it.org/local-it/mitgliederverwaltung - username: - from_secret: DRONE_REGISTRY_USERNAME - password: - from_secret: DRONE_REGISTRY_TOKEN - tags: - - latest - - ${DRONE_COMMIT_SHA:0:8} - when: - event: - - push - -depends_on: - - check-fast - ---- -kind: pipeline -type: docker -name: build-and-release - -trigger: - event: - - tag - -steps: - - name: build-and-publish-container - image: plugins/docker - settings: - registry: git.local-it.org - repo: git.local-it.org/local-it/mitgliederverwaltung - username: - from_secret: DRONE_REGISTRY_USERNAME - password: - from_secret: DRONE_REGISTRY_TOKEN - auto_tag: true - when: - event: - - tag - -depends_on: - - check-fast - ---- -kind: pipeline -type: docker -name: renovate - -trigger: - event: - - cron - - custom - branch: - - main - -environment: - LOG_LEVEL: debug - -steps: - - name: renovate - image: renovate/renovate:43.165 - environment: - RENOVATE_CONFIG_FILE: "renovate_backend_config.js" - RENOVATE_TOKEN: - from_secret: RENOVATE_TOKEN - GITHUB_COM_TOKEN: - from_secret: GITHUB_COM_TOKEN - commands: - # https://github.com/renovatebot/renovate/discussions/15049 - - unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL - - renovate-config-validator - - renovate -- 2.47.2 From 263857ee26c3ec0cebd92d0e64e956505a10b5de Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 1 Jun 2026 23:45:07 +0200 Subject: [PATCH 02/16] feat(dialyzer): add typecheck stage to full CI pipelines --- .dialyzer_ignore.exs | 11 +++++++++++ .drone.jsonnet | 19 +++++++++++++++++-- .gitignore | 4 ++++ Justfile | 15 +++++++++++++++ mix.exs | 19 +++++++++++++++++++ mix.lock | 2 ++ 6 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 .dialyzer_ignore.exs diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs new file mode 100644 index 0000000..c89978c --- /dev/null +++ b/.dialyzer_ignore.exs @@ -0,0 +1,11 @@ +# Dialyzer ignore list. +# +# This file is for PROVEN false positives only. Each entry must carry a +# `# why:` comment explaining why Dialyzer is wrong about the call site. +# Real findings get fixed by adjusting @spec, return types, or pattern +# matches — never silenced here. +# +# Format: each entry is either a path string, a {path, warning} tuple, +# or a {path, warning, line} tuple. See: +# https://hexdocs.pm/dialyxir/readme.html#elixir-format +[] diff --git a/.drone.jsonnet b/.drone.jsonnet index ba8bacd..388e8f4 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -27,7 +27,7 @@ local step_compute_cache = { local step_restore_cache = { name: 'restore-cache', image: 'drillster/drone-volume-cache', - settings: { restore: true, mount: ['./deps', './_build'], ttl: 30 }, + settings: { restore: true, mount: ['./deps', './_build', './priv/plts'], ttl: 30 }, volumes: cache_mount, }; @@ -47,6 +47,20 @@ local step_lint = { ], }; +local step_typecheck = { + name: 'typecheck', + image: elixir, + commands: [ + 'mix local.hex --force', + 'mix deps.get', + 'mkdir -p priv/plts', + // Build/refresh PLT — no-op on cache hit, full build (5-15 min) on cache miss. + 'mix dialyzer --plt', + // Actual typecheck. --format short keeps log noise down on red builds. + 'mix dialyzer --format short', + ], +}; + local step_wait_postgres = { name: 'wait_for_postgres', image: postgres_image, @@ -69,7 +83,7 @@ local step_wait_postgres = { local step_rebuild_cache = { name: 'rebuild-cache', image: 'drillster/drone-volume-cache', - settings: { rebuild: true, mount: ['./deps', './_build'] }, + settings: { rebuild: true, mount: ['./deps', './_build', './priv/plts'] }, volumes: cache_mount, }; @@ -99,6 +113,7 @@ local check_pipeline(name, trigger, test) = { step_compute_cache, step_restore_cache, step_lint, + ] + (if test.name == 'test-all' then [step_typecheck] else []) + [ step_wait_postgres, test, step_rebuild_cache, diff --git a/.gitignore b/.gitignore index b9096bd..14620df 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,7 @@ notes.md # Do NOT commit these — they are local to the dev machine .pipeline/ .claude/ + +# Dialyzer PLT files — built locally and in CI cache, never tracked. +/priv/plts/*.plt +/priv/plts/*.plt.hash diff --git a/Justfile b/Justfile index e0bd0d3..54c395f 100644 --- a/Justfile +++ b/Justfile @@ -31,6 +31,21 @@ start-database: ci-dev: lint audit test-fast +# Build the Dialyzer PLT. Idempotent — no-op once the PLT is up to date. +# First build takes 5–15 min; subsequent runs are seconds. PLT files live in +# priv/plts/ and are gitignored. +plt: install-dependencies + @mkdir -p priv/plts + mix dialyzer --plt + +# Typecheck via Dialyzer. Slow stage, NOT part of ci-dev. +typecheck: plt + mix dialyzer --format short + +# Full CI: inner loop plus typecheck. Use locally before pushing; Drone CI +# runs equivalent steps with PLT caching. +ci: ci-dev typecheck + gettext: mix gettext.extract mix gettext.merge priv/gettext --on-obsolete=mark_as_obsolete diff --git a/mix.exs b/mix.exs index c037e46..a510a7e 100644 --- a/mix.exs +++ b/mix.exs @@ -12,6 +12,7 @@ defmodule Mv.MixProject do compilers: [:phoenix_live_view] ++ Mix.compilers(), aliases: aliases(), deps: deps(), + dialyzer: dialyzer(), listeners: [Phoenix.CodeReloader], gettext: [write_reference_line_numbers: false] ] @@ -80,6 +81,7 @@ defmodule Mv.MixProject do {:sobelow, "~> 0.14", only: [:dev, :test], runtime: false}, {:bypass, "~> 2.1", only: [:dev, :test]}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, + {:dialyxir, "~> 1.4", only: [:dev, :test], runtime: false}, {:picosat_elixir, "~> 0.1"}, {:ecto_commons, "~> 0.3"}, {:slugify, "~> 1.3"}, @@ -112,4 +114,21 @@ defmodule Mv.MixProject do "phx.routes": ["phx.routes", "ash_authentication.phoenix.routes"] ] end + + defp dialyzer do + [ + plt_file: {:no_warn, "priv/plts/dialyzer.plt"}, + plt_core_path: "priv/plts/core.plt", + plt_add_apps: [:mix, :ex_unit], + flags: [ + :error_handling, + :unmatched_returns, + :extra_return, + :missing_return, + :underspecs + ], + ignore_warnings: ".dialyzer_ignore.exs", + list_unused_filters: true + ] + end end diff --git a/mix.lock b/mix.lock index 0a36e9e..7dd592f 100644 --- a/mix.lock +++ b/mix.lock @@ -23,11 +23,13 @@ "crux": {:hex, :crux, "0.1.2", "4441c9e3a34f1e340954ce96b9ad5a2de13ceb4f97b3f910211227bb92e2ca90", [:mix], [{:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:stream_data, "~> 1.0", [hex: :stream_data, repo: "hexpm", optional: true]}], "hexpm", "563ea3748ebfba9cc078e6d198a1d6a06015a8fae503f0b721363139f0ddb350"}, "db_connection": {:hex, :db_connection, "2.10.1", "d5465f6bcc125c1b8981c1dbf23c193ca16f446ec0b25832dc174f74f18be510", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "18ed94c6e627b4bf452dbd4df61b69a35a1e768525140bc1917b7a685026a6a3"}, "decimal": {:hex, :decimal, "3.1.0", "9ede268cff827e6f0c4fb1b34747c82630dce5d7b877dfb22ec8f0cb25855fce", [:mix], [], "hexpm", "e8b3efb3bb3a13cb5e4268ffe128569067b1972e9dee013537c71a5b073168f9"}, + "dialyxir": {:hex, :dialyxir, "1.4.7", "dda948fcee52962e4b6c5b4b16b2d8fa7d50d8645bbae8b8685c3f9ecb7f5f4d", [:mix], [{:erlex, ">= 0.2.8", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "b34527202e6eb8cee198efec110996c25c5898f43a4094df157f8d28f27d9efe"}, "dns_cluster": {:hex, :dns_cluster, "0.2.0", "aa8eb46e3bd0326bd67b84790c561733b25c5ba2fe3c7e36f28e88f384ebcb33", [:mix], [], "hexpm", "ba6f1893411c69c01b9e8e8f772062535a4cf70f3f35bcc964a324078d8c8240"}, "ecto": {:hex, :ecto, "3.13.6", "352135b474f91d1ab99a1b502171d207e9db60421c9e3d0ecab4c7ab96b24d14", [:mix], [{:decimal, "~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "8afa059bc16cd2c94739ec0a11e3e5df69d828125119109bef35f20a21a76af2"}, "ecto_commons": {:hex, :ecto_commons, "0.3.7", "f33c162a6f63695d5939af02c65a0e76aa6e7278b82c7bfc357ffbfea353bf0f", [:mix], [{:burnex, "~> 3.0", [hex: :burnex, repo: "hexpm", optional: true]}, {:ecto, "~> 3.4", [hex: :ecto, repo: "hexpm", optional: false]}, {:ex_phone_number, "~> 0.4", [hex: :ex_phone_number, repo: "hexpm", optional: false]}, {:luhn, "~> 0.3.0", [hex: :luhn, repo: "hexpm", optional: false]}], "hexpm", "9c33771ebd38cd83d3f90fab6069826ba9d4f7580f1481b3c0913f8b9795c5fd"}, "ecto_sql": {:hex, :ecto_sql, "3.13.5", "2f8282b2ad97bf0f0d3217ea0a6fff320ead9e2f8770f810141189d182dc304e", [:mix], [{:db_connection, "~> 2.4.1 or ~> 2.5", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.13.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:myxql, "~> 0.7", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.19 or ~> 1.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:tds, "~> 2.1.1 or ~> 2.2", [hex: :tds, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "aa36751f4e6a2b56ae79efb0e088042e010ff4935fc8684e74c23b1f49e25fdc"}, "elixir_make": {:hex, :elixir_make, "0.9.0", "6484b3cd8c0cee58f09f05ecaf1a140a8c97670671a6a0e7ab4dc326c3109726", [:mix], [], "hexpm", "db23d4fd8b757462ad02f8aa73431a426fe6671c80b200d9710caf3d1dd0ffdb"}, + "erlex": {:hex, :erlex, "0.2.9", "7debbbaa9f4f368b8cd648983e0f1d7963028508e9c59e9d4ed504e94ef52a55", [:mix], [], "hexpm", "8cfffc0ec7159e6d73de2ab28a588064de80f88b2798d5cbe4482cbbc200178b"}, "esbuild": {:hex, :esbuild, "0.10.0", "b0aa3388a1c23e727c5a3e7427c932d89ee791746b0081bbe56103e9ef3d291f", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "468489cda427b974a7cc9f03ace55368a83e1a7be12fba7e30969af78e5f8c70"}, "ets": {:hex, :ets, "0.9.0", "79c6a6c205436780486f72d84230c6cba2f8a9920456750ddd1e47389107d5fd", [:mix], [], "hexpm", "2861fdfb04bcaeff370f1a5904eec864f0a56dcfebe5921ea9aadf2a481c822b"}, "ex_phone_number": {:hex, :ex_phone_number, "0.4.10", "11809f6600b2ecb0a2e75d496c2ec2f273d49d1e2f58b2be2667decb0aabfb43", [:mix], [{:sweet_xml, "~> 0.7", [hex: :sweet_xml, repo: "hexpm", optional: false]}], "hexpm", "eefccf58d8149d64af658721bff0edcb9e9b8943f74000ede151948ef03046c1"}, -- 2.47.2 From fd8e6ac178c00fb4e4d061c87a8cb436ce7fe856 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 11:25:03 +0200 Subject: [PATCH 03/16] refactor(types): reconcile @specs with their success typings --- lib/membership/membership.ex | 5 +++- lib/mv/authorization/permission_sets.ex | 5 ++-- lib/mv/config.ex | 16 +++++++---- lib/mv/helpers/system_actor.ex | 28 +++++++++++-------- lib/mv/membership/import/csv_parser.ex | 8 ++++-- lib/mv/membership/member_export.ex | 17 ++++++++++- lib/mv/membership/members_csv.ex | 2 +- .../membership_fees/cycle_generation_job.ex | 8 +++--- lib/mv/membership_fees/cycle_generator.ex | 10 ++++++- lib/mv/vereinfacht/client.ex | 10 +++++-- lib/mv/vereinfacht/vereinfacht.ex | 2 +- .../member_live/index/field_visibility.ex | 2 +- .../live/membership_fee_type_live/form.ex | 2 +- lib/mv_web/live/role_live/form.ex | 2 +- lib/mv_web/live/user_live/form.ex | 4 +-- lib/mv_web/live_helpers.ex | 5 +++- lib/mv_web/translations/field_types.ex | 4 ++- 17 files changed, 92 insertions(+), 38 deletions(-) diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 7fa35dc..72be69b 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -836,7 +836,10 @@ defmodule Mv.Membership do - `{:ok, rejected_request}` - Rejected JoinRequest - `{: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 actor = Keyword.get(opts, :actor) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 3ffae93..ae84cdb 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -43,6 +43,7 @@ defmodule Mv.Authorization.PermissionSets do 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 action :: :read | :create | :update | :destroy @@ -88,7 +89,7 @@ defmodule Mv.Authorization.PermissionSets do iex> PermissionSets.all_permission_sets() [:own_data, :read_only, :normal_user, :admin] """ - @spec all_permission_sets() :: [atom()] + @spec all_permission_sets() :: [permission_set_name(), ...] def all_permission_sets do [:own_data, :read_only, :normal_user, :admin] end @@ -107,7 +108,7 @@ defmodule Mv.Authorization.PermissionSets do iex> PermissionSets.get_permissions(:invalid) ** (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 raise ArgumentError, diff --git a/lib/mv/config.ex b/lib/mv/config.ex index 870d1d3..f198191 100644 --- a/lib/mv/config.ex +++ b/lib/mv/config.ex @@ -409,7 +409,7 @@ defmodule Mv.Config do @doc """ 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 case env_or_setting("OIDC_GROUPS_CLAIM", :oidc_groups_claim) do nil -> "groups" @@ -492,7 +492,7 @@ defmodule Mv.Config do - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_PORT` - Settings mode: read from Settings only """ - @spec smtp_port() :: non_neg_integer() | nil + @spec smtp_port() :: pos_integer() | nil def smtp_port do if smtp_env_mode?() do parse_smtp_port_env(System.get_env("SMTP_PORT")) @@ -638,9 +638,15 @@ defmodule Mv.Config do """ @spec mail_from_name() :: String.t() def mail_from_name do - case System.get_env("MAIL_FROM_NAME") do - nil -> get_from_settings(:smtp_from_name) || "Mila" - value -> trim_nil(value) || "Mila" + name = + case System.get_env("MAIL_FROM_NAME") do + nil -> get_from_settings(:smtp_from_name) + value -> trim_nil(value) + end + + case name do + nil -> "Mila" + name -> name end end diff --git a/lib/mv/helpers/system_actor.ex b/lib/mv/helpers/system_actor.ex index 8cd93d2..7b86a3c 100644 --- a/lib/mv/helpers/system_actor.ex +++ b/lib/mv/helpers/system_actor.ex @@ -225,7 +225,10 @@ defmodule Mv.Helpers.SystemActor do # This allows configuration via SYSTEM_ACTOR_EMAIL env var @spec system_user_email_config() :: String.t() 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 # Loads the system actor from the database @@ -257,7 +260,7 @@ defmodule Mv.Helpers.SystemActor do end # 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 case load_admin_user_fallback() do {:ok, admin_user} -> @@ -393,15 +396,18 @@ defmodule Mv.Helpers.SystemActor do # 1. Only creates system user with known email # 2. Only called during system actor initialization (bootstrap) # 3. Once created, all subsequent operations use proper authorization - Accounts.create_user!(%{email: system_user_email_config()}, - upsert?: true, - upsert_identity: :unique_email, - authorize?: false - ) - |> Ash.Changeset.for_update(:update_internal, %{}) - |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) - |> Ash.update!(authorize?: false) - |> Ash.load!(:role, domain: Mv.Accounts, authorize?: false) + user = + Accounts.create_user!(%{email: system_user_email_config()}, + upsert?: true, + upsert_identity: :unique_email, + authorize?: false + ) + |> Ash.Changeset.for_update(:update_internal, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + |> Ash.load!(:role, domain: Mv.Accounts, authorize?: false) + + %Accounts.User{} = user end # Finds a user by email address diff --git a/lib/mv/membership/import/csv_parser.ex b/lib/mv/membership/import/csv_parser.ex index 2de75ee..142450f 100644 --- a/lib/mv/membership/import/csv_parser.ex +++ b/lib/mv/membership/import/csv_parser.ex @@ -100,7 +100,8 @@ defmodule Mv.Membership.Import.CsvParser do |> String.replace("\r", "\n") 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.CsvParserComma 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: "," 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 case parse_single_record(parser, header_record, nil) do {:ok, fields} -> Enum.count(fields, &(String.trim(&1) != "")) diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex index 16341c4..d96c82f 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -16,6 +16,21 @@ defmodule Mv.Membership.MemberExport do alias MvWeb.MemberLive.Index 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)) ++ ["membership_fee_type", "membership_fee_status", "groups"] @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, :show_current_cycle, :cycle_status_filter, :boolean_filters. """ - @spec parse_params(map()) :: map() + @spec parse_params(map()) :: parsed_params() def parse_params(params) do # DB fields come from "member_fields" raw_member_fields = extract_list(params, "member_fields") diff --git a/lib/mv/membership/members_csv.ex b/lib/mv/membership/members_csv.ex index 6331893..0a19810 100644 --- a/lib/mv/membership/members_csv.ex +++ b/lib/mv/membership/members_csv.ex @@ -21,7 +21,7 @@ defmodule Mv.Membership.MembersCSV do Returns iodata suitable for `IO.iodata_to_binary/1` or sending as response body. 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 header = build_header(columns) rows = Enum.map(members, fn member -> build_row(member, columns) end) diff --git a/lib/mv/membership_fees/cycle_generation_job.ex b/lib/mv/membership_fees/cycle_generation_job.ex index 71a3158..b38886c 100644 --- a/lib/mv/membership_fees/cycle_generation_job.ex +++ b/lib/mv/membership_fees/cycle_generation_job.ex @@ -58,7 +58,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do {: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 Logger.info("Starting membership fee cycle generation job") start_time = System.monotonic_time(:millisecond) @@ -98,7 +98,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do 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 Logger.info("Starting membership fee cycle generation job with opts: #{inspect(opts)}") start_time = System.monotonic_time(:millisecond) @@ -135,7 +135,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do - `{: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 today = Date.utc_today() @@ -166,7 +166,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do - `{: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 Logger.info("Generating cycles for member #{member_id}") CycleGenerator.generate_cycles_for_member(member_id) diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 8f1bc7c..4014d80 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -1,4 +1,11 @@ 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 """ Module for generating membership fee cycles for members. @@ -159,7 +166,8 @@ defmodule Mv.MembershipFees.CycleGenerator do - `{: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 today = Keyword.get(opts, :today, Date.utc_today()) batch_size = Keyword.get(opts, :batch_size, 10) diff --git a/lib/mv/vereinfacht/client.ex b/lib/mv/vereinfacht/client.ex index 3cbba71..6a81c46 100644 --- a/lib/mv/vereinfacht/client.ex +++ b/lib/mv/vereinfacht/client.ex @@ -8,6 +8,12 @@ defmodule Mv.Vereinfacht.Client do """ 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" @doc """ @@ -31,7 +37,7 @@ defmodule Mv.Vereinfacht.Client do {:error, :not_configured} """ @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 if blank?(api_url) or blank?(api_key) or blank?(club_id) do {:error, :not_configured} @@ -230,7 +236,7 @@ defmodule Mv.Vereinfacht.Client do 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 fetch_contact(contact_id, []) end diff --git a/lib/mv/vereinfacht/vereinfacht.ex b/lib/mv/vereinfacht/vereinfacht.ex index 83492b7..4d58f8d 100644 --- a/lib/mv/vereinfacht/vereinfacht.ex +++ b/lib/mv/vereinfacht/vereinfacht.ex @@ -26,7 +26,7 @@ defmodule Mv.Vereinfacht do - `{:error, {:http, status, message}}` – API returned an error (e.g. 401, 403) - `{: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 Client.test_connection( Mv.Config.vereinfacht_api_url(), diff --git a/lib/mv_web/live/member_live/index/field_visibility.ex b/lib/mv_web/live/member_live/index/field_visibility.ex index df20d25..52ebe86 100644 --- a/lib/mv_web/live/member_live/index/field_visibility.ex +++ b/lib/mv_web/live/member_live/index/field_visibility.ex @@ -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 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 @doc """ diff --git a/lib/mv_web/live/membership_fee_type_live/form.ex b/lib/mv_web/live/membership_fee_type_live/form.ex index bfdfa2d..ebaa977 100644 --- a/lib/mv_web/live/membership_fee_type_live/form.ex +++ b/lib/mv_web/live/membership_fee_type_live/form.ex @@ -341,7 +341,7 @@ defmodule MvWeb.MembershipFeeTypeLive.Form do end end - @spec notify_parent(any()) :: any() + @spec notify_parent(any()) :: {module(), any()} defp notify_parent(msg), do: send(self(), {__MODULE__, msg}) @spec assign_form(Phoenix.LiveView.Socket.t()) :: Phoenix.LiveView.Socket.t() diff --git a/lib/mv_web/live/role_live/form.ex b/lib/mv_web/live/role_live/form.ex index 51f5cac..eb672da 100644 --- a/lib/mv_web/live/role_live/form.ex +++ b/lib/mv_web/live/role_live/form.ex @@ -186,7 +186,7 @@ defmodule MvWeb.RoleLive.Form do end end - @spec notify_parent(any()) :: any() + @spec notify_parent(any()) :: {module(), any()} defp notify_parent(msg), do: send(self(), {__MODULE__, msg}) @spec assign_form(Phoenix.LiveView.Socket.t()) :: Phoenix.LiveView.Socket.t() diff --git a/lib/mv_web/live/user_live/form.ex b/lib/mv_web/live/user_live/form.ex index 4a26078..35ce1fe 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -775,7 +775,7 @@ defmodule MvWeb.UserLive.Form do )} end - @spec notify_parent(any()) :: any() + @spec notify_parent(any()) :: {module(), any()} defp notify_parent(msg), do: send(self(), {__MODULE__, msg}) # 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) 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 case Authorization.list_roles(actor: actor) do {:ok, roles} -> roles diff --git a/lib/mv_web/live_helpers.ex b/lib/mv_web/live_helpers.ex index 4206aa6..ebf51e2 100644 --- a/lib/mv_web/live_helpers.ex +++ b/lib/mv_web/live_helpers.ex @@ -145,7 +145,10 @@ defmodule MvWeb.LiveHelpers do end """ @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 AshPhoenix.Form.submit(form, params: params, action_opts: ash_actor_opts(actor)) end diff --git a/lib/mv_web/translations/field_types.ex b/lib/mv_web/translations/field_types.ex index 969f20b..1580b99 100644 --- a/lib/mv_web/translations/field_types.ex +++ b/lib/mv_web/translations/field_types.ex @@ -12,7 +12,9 @@ defmodule MvWeb.Translations.FieldTypes do """ 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(:integer), do: gettext("Number") def label(:boolean), do: gettext("Yes/No-Selection") -- 2.47.2 From 5352a635c6a8584e0df24a46e5210b2818813b02 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 11:33:14 +0200 Subject: [PATCH 04/16] refactor(release): bind discarded results of side-effecting release tasks --- lib/mv/release.ex | 53 +++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/mv/release.ex b/lib/mv/release.ex index 116b276..5db4751 100644 --- a/lib/mv/release.ex +++ b/lib/mv/release.ex @@ -22,7 +22,7 @@ defmodule Mv.Release do require Logger def migrate do - load_app() + _ = load_app() for repo <- repos() do {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :up, all: true)) @@ -75,14 +75,14 @@ defmodule Mv.Release do dev_path = Path.join(priv, "repo/seeds_dev.exs") prev = Code.compiler_options() - Code.compiler_options(ignore_module_conflict: true) + _ = Code.compiler_options(ignore_module_conflict: true) try do - Code.eval_file(bootstrap_path) + _ = Code.eval_file(bootstrap_path) IO.puts("✅ Bootstrap seeds completed.") if System.get_env("RUN_DEV_SEEDS") == "true" do - Code.eval_file(dev_path) + _ = Code.eval_file(dev_path) IO.puts("✅ Dev seeds completed.") end after @@ -92,7 +92,7 @@ defmodule Mv.Release do end def rollback(repo, version) do - load_app() + _ = load_app() {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :down, to: version)) end @@ -139,10 +139,11 @@ defmodule Mv.Release do {:ok, %Role{} = admin_role} -> case get_user_by_email(email) do {:ok, %User{} = user} -> - user - |> Ash.Changeset.for_update(:update, %{}) - |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) - |> Ash.update!(authorize?: false) + _ = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) :ok @@ -189,15 +190,16 @@ defmodule Mv.Release do defp create_admin_user(email, password, admin_role) do case Accounts.create_user(%{email: email}, authorize?: false) do {:ok, user} -> - user - |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) - |> Ash.update!(authorize?: false) - |> then(fn u -> - u - |> Ash.Changeset.for_update(:update, %{}) - |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + _ = + user + |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) |> Ash.update!(authorize?: false) - end) + |> then(fn u -> + u + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + end) :ok @@ -207,15 +209,16 @@ defmodule Mv.Release do end defp update_admin_user(user, password, admin_role) do - user - |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) - |> Ash.update!(authorize?: false) - |> then(fn u -> - u - |> Ash.Changeset.for_update(:update, %{}) - |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + _ = + user + |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) |> Ash.update!(authorize?: false) - end) + |> then(fn u -> + u + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + end) :ok end -- 2.47.2 From 04ab05f556daf6e60bf36ae312ec7b0c3aaa1ba6 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 11:39:04 +0200 Subject: [PATCH 05/16] fix(member-export): forbid request without actor instead of falling through The nil-actor guard used a one-armed if and continued into the export path regardless. The CheckPagePermission plug already halts unauthenticated requests before this controller runs, so the corrected early return preserves observable behavior while removing the dead fall-through. The export action is split into per-payload clauses so the guard reads as a flat early return. --- .../controllers/member_export_controller.ex | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/lib/mv_web/controllers/member_export_controller.ex b/lib/mv_web/controllers/member_export_controller.ex index 9b08f5d..e9c4a2a 100644 --- a/lib/mv_web/controllers/member_export_controller.ex +++ b/lib/mv_web/controllers/member_export_controller.ex @@ -25,31 +25,33 @@ defmodule MvWeb.MemberExportController do @custom_field_prefix Mv.Constants.custom_field_prefix() def export(conn, params) do - actor = current_actor(conn) - if is_nil(actor), do: return_forbidden(conn) - - case params["payload"] do - nil -> - conn - |> put_status(400) - |> put_resp_content_type("application/json") - |> json(%{error: "payload required"}) - - payload when is_binary(payload) -> - case Jason.decode(payload) do - {:ok, decoded} when is_map(decoded) -> - parsed = parse_and_validate(decoded) - run_export(conn, actor, parsed) - - _ -> - conn - |> put_status(400) - |> put_resp_content_type("application/json") - |> json(%{error: "invalid JSON"}) - end + case current_actor(conn) do + nil -> return_forbidden(conn) + actor -> export_with_actor(conn, actor, params["payload"]) end end + defp export_with_actor(conn, actor, payload) when is_binary(payload) do + case Jason.decode(payload) do + {:ok, decoded} when is_map(decoded) -> + run_export(conn, actor, parse_and_validate(decoded)) + + _ -> + json_error(conn, "invalid JSON") + end + end + + defp export_with_actor(conn, _actor, _payload) do + json_error(conn, "payload required") + end + + defp json_error(conn, message) do + conn + |> put_status(400) + |> put_resp_content_type("application/json") + |> json(%{error: message}) + end + defp current_actor(conn) do conn.assigns[:current_user] |> Actor.ensure_loaded() -- 2.47.2 From 848f0cd013832c3e153f4fdff17db2c97d8314a0 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 11:42:57 +0200 Subject: [PATCH 06/16] refactor(types): bind intentionally discarded side-effecting results --- lib/membership/member.ex | 8 ++++---- .../member/changes/unrelate_user_when_argument_nil.ex | 7 ++++--- lib/mv/membership_fees/cycle_generator.ex | 4 ++-- lib/mv/vereinfacht/sync_flash.ex | 7 ++++--- lib/mv_web/live/auth/sign_in_live.ex | 4 ++-- lib/mv_web/live/auth/sign_out_live.ex | 4 ++-- lib/mv_web/live/global_settings_live.ex | 2 +- lib/mv_web/live/membership_fee_type_live/form.ex | 2 +- lib/mv_web/live/role_live/form.ex | 2 +- lib/mv_web/live/user_live/form.ex | 2 +- lib/mv_web/live_helpers.ex | 2 +- lib/mv_web/router.ex | 2 +- 12 files changed, 24 insertions(+), 22 deletions(-) diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 85f5562..4959e78 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -939,7 +939,7 @@ defmodule Mv.Membership.Member do # Already in transaction: use advisory lock directly # Returns {:ok, notifications} - notifications should be returned to after_action hook defp regenerate_cycles_in_transaction(member, today, lock_key) do - EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + _ = EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) end @@ -947,7 +947,7 @@ defmodule Mv.Membership.Member do # Returns {:ok, notifications} - notifications should be sent by caller (e.g., via after_action) defp regenerate_cycles_new_transaction(member, today, lock_key) do Repo.transaction(fn -> - EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + _ = EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) do {:ok, notifications} -> @@ -1093,7 +1093,7 @@ defmodule Mv.Membership.Member do initiator: initiator ) do {:ok, cycles, notifications} -> - send_notifications_if_any(notifications) + _ = send_notifications_if_any(notifications) log_cycle_generation_success(member, cycles, notifications, sync: true, @@ -1112,7 +1112,7 @@ defmodule Mv.Membership.Member do initiator: initiator ) do {:ok, cycles, notifications} -> - send_notifications_if_any(notifications) + _ = send_notifications_if_any(notifications) log_cycle_generation_success(member, cycles, notifications, sync: false, diff --git a/lib/membership/member/changes/unrelate_user_when_argument_nil.ex b/lib/membership/member/changes/unrelate_user_when_argument_nil.ex index dc4d097..da8a291 100644 --- a/lib/membership/member/changes/unrelate_user_when_argument_nil.ex +++ b/lib/membership/member/changes/unrelate_user_when_argument_nil.ex @@ -37,9 +37,10 @@ defmodule Mv.Membership.Member.Changes.UnrelateUserWhenArgumentNil do {:ok, %{user: user}} when not is_nil(user) -> # User's :update action only accepts [:email]; use :update_user so # manage_relationship(:member, ..., on_missing: :unrelate) runs and clears member_id. - user - |> Ash.Changeset.for_update(:update_user, %{member: nil}, domain: Mv.Accounts) - |> Ash.update(domain: Mv.Accounts, actor: actor, authorize?: false) + _ = + user + |> Ash.Changeset.for_update(:update_user, %{member: nil}, domain: Mv.Accounts) + |> Ash.update(domain: Mv.Accounts, actor: actor, authorize?: false) changeset diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 4014d80..189f40a 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -122,7 +122,7 @@ defmodule Mv.MembershipFees.CycleGenerator do lock_key = Member.advisory_lock_key_for_member_id(member.id) Repo.transaction(fn -> - EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + _ = EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) case do_generate_cycles(member, today, opts) do {:ok, cycles, notifications} -> @@ -220,7 +220,7 @@ defmodule Mv.MembershipFees.CycleGenerator do defp process_member_cycle_generation(member, today) do case generate_cycles_for_member(member, today: today) do {:ok, _cycles, notifications} = ok -> - send_notifications_for_batch_job(notifications) + _ = send_notifications_for_batch_job(notifications) {member.id, ok} {:error, _reason} = err -> diff --git a/lib/mv/vereinfacht/sync_flash.ex b/lib/mv/vereinfacht/sync_flash.ex index 874a717..5c643b6 100644 --- a/lib/mv/vereinfacht/sync_flash.ex +++ b/lib/mv/vereinfacht/sync_flash.ex @@ -37,9 +37,10 @@ defmodule Mv.Vereinfacht.SyncFlash do def create_table! do # :public so any process can write (SyncContact runs in LiveView/Ash transaction process, # not the process that created the table). :protected would restrict writes to the creating process. - if :ets.whereis(@table) == :undefined do - :ets.new(@table, [:set, :public, :named_table]) - end + _ = + if :ets.whereis(@table) == :undefined do + :ets.new(@table, [:set, :public, :named_table]) + end :ok end diff --git a/lib/mv_web/live/auth/sign_in_live.ex b/lib/mv_web/live/auth/sign_in_live.ex index fb41f1b..c519914 100644 --- a/lib/mv_web/live/auth/sign_in_live.ex +++ b/lib/mv_web/live/auth/sign_in_live.ex @@ -30,8 +30,8 @@ defmodule MvWeb.SignInLive do # Set both backend-specific and global locale so Gettext.get_locale/0 and # Gettext.get_locale/1 both return the correct value (important for the # language-selector `selected` attribute in Layouts.public_page). - Gettext.put_locale(MvWeb.Gettext, locale) - Gettext.put_locale(locale) + _ = Gettext.put_locale(MvWeb.Gettext, locale) + _ = Gettext.put_locale(locale) # Prepend DE-specific overrides when locale is German so that components # without _gettext support (e.g. HorizontalRule) still render in German. diff --git a/lib/mv_web/live/auth/sign_out_live.ex b/lib/mv_web/live/auth/sign_out_live.ex index 2a0e0df..569337a 100644 --- a/lib/mv_web/live/auth/sign_out_live.ex +++ b/lib/mv_web/live/auth/sign_out_live.ex @@ -16,8 +16,8 @@ defmodule MvWeb.SignOutLive do @impl true def mount(_params, session, socket) do locale = session["locale"] || Application.get_env(:mv, :default_locale, "de") - Gettext.put_locale(MvWeb.Gettext, locale) - Gettext.put_locale(locale) + _ = Gettext.put_locale(MvWeb.Gettext, locale) + _ = Gettext.put_locale(locale) club_name = case Membership.get_settings() do diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index 6a1c926..6a456fe 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -52,7 +52,7 @@ defmodule MvWeb.GlobalSettingsLive do # Get locale from session; same fallback as router/LiveUserAuth (respects config :default_locale in test) locale = session["locale"] || Application.get_env(:mv, :default_locale, "de") - Gettext.put_locale(MvWeb.Gettext, locale) + _ = Gettext.put_locale(MvWeb.Gettext, locale) actor = MvWeb.LiveHelpers.current_actor(socket) custom_fields = load_custom_fields(actor) diff --git a/lib/mv_web/live/membership_fee_type_live/form.ex b/lib/mv_web/live/membership_fee_type_live/form.ex index ebaa977..a4d8506 100644 --- a/lib/mv_web/live/membership_fee_type_live/form.ex +++ b/lib/mv_web/live/membership_fee_type_live/form.ex @@ -326,7 +326,7 @@ defmodule MvWeb.MembershipFeeTypeLive.Form do case submit_form(socket.assigns.form, params, actor) do {:ok, membership_fee_type} -> - notify_parent({:saved, membership_fee_type}) + _ = notify_parent({:saved, membership_fee_type}) socket = socket diff --git a/lib/mv_web/live/role_live/form.ex b/lib/mv_web/live/role_live/form.ex index eb672da..2e315b9 100644 --- a/lib/mv_web/live/role_live/form.ex +++ b/lib/mv_web/live/role_live/form.ex @@ -165,7 +165,7 @@ defmodule MvWeb.RoleLive.Form do case MvWeb.LiveHelpers.submit_form(socket.assigns.form, role_params, actor) do {:ok, role} -> - notify_parent({:saved, role}) + _ = notify_parent({:saved, role}) redirect_path = if socket.assigns.return_to == "show" do diff --git a/lib/mv_web/live/user_live/form.ex b/lib/mv_web/live/user_live/form.ex index 35ce1fe..622228d 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -734,7 +734,7 @@ defmodule MvWeb.UserLive.Form do end defp handle_save_success(socket, updated_user) do - notify_parent({:saved, updated_user}) + _ = notify_parent({:saved, updated_user}) action = get_action_name(socket.assigns.form.source.type) diff --git a/lib/mv_web/live_helpers.ex b/lib/mv_web/live_helpers.ex index ebf51e2..003c36c 100644 --- a/lib/mv_web/live_helpers.ex +++ b/lib/mv_web/live_helpers.ex @@ -22,7 +22,7 @@ defmodule MvWeb.LiveHelpers do def on_mount(:default, _params, session, socket) do locale = session["locale"] || "de" - Gettext.put_locale(locale) + _ = Gettext.put_locale(locale) # Browser timezone from LiveSocket connect params (set in app.js via Intl API) connect_params = socket.private[:connect_params] || %{} diff --git a/lib/mv_web/router.ex b/lib/mv_web/router.ex index f42eb29..64036c9 100644 --- a/lib/mv_web/router.ex +++ b/lib/mv_web/router.ex @@ -188,7 +188,7 @@ defmodule MvWeb.Router do get_locale_from_cookie(conn) || extract_locale_from_headers(conn.req_headers) - Gettext.put_locale(MvWeb.Gettext, locale) + _ = Gettext.put_locale(MvWeb.Gettext, locale) conn |> put_session(:locale, locale) -- 2.47.2 From c0395f16e874a4ea9d7a9b02b8df3d3d8519c654 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 11:46:54 +0200 Subject: [PATCH 07/16] fix(types): resolve unknown type references in member and authorization specs --- lib/membership/member.ex | 9 ++++++--- lib/mv_web/authorization.ex | 3 +-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 4959e78..bf8a48c 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -51,6 +51,9 @@ defmodule Mv.Membership.Member do require Logger + @typedoc "An `Mv.Membership.Member` resource record." + @type t :: %__MODULE__{} + # Module constants @member_search_limit 10 @@ -791,7 +794,7 @@ defmodule Mv.Membership.Member do # nil/[] when membership_fee_type is missing. @doc false - @spec get_current_cycle(Member.t()) :: MembershipFeeCycle.t() | nil + @spec get_current_cycle(t()) :: MembershipFeeCycle.t() | nil def get_current_cycle(member) do today = Date.utc_today() @@ -821,7 +824,7 @@ defmodule Mv.Membership.Member do end @doc false - @spec get_last_completed_cycle(Member.t()) :: MembershipFeeCycle.t() | nil + @spec get_last_completed_cycle(t()) :: MembershipFeeCycle.t() | nil def get_last_completed_cycle(member) do today = Date.utc_today() @@ -867,7 +870,7 @@ defmodule Mv.Membership.Member do end @doc false - @spec get_overdue_cycles(Member.t()) :: [MembershipFeeCycle.t()] + @spec get_overdue_cycles(t()) :: [MembershipFeeCycle.t()] def get_overdue_cycles(member) do today = Date.utc_today() diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex index d821416..de009b6 100644 --- a/lib/mv_web/authorization.ex +++ b/lib/mv_web/authorization.ex @@ -113,8 +113,7 @@ defmodule MvWeb.Authorization do iex> can_access_page?(mitglied, "/members") false """ - @spec can_access_page?(map() | nil, String.t() | Phoenix.VerifiedRoutes.unverified_path()) :: - boolean() + @spec can_access_page?(map() | nil, String.t()) :: boolean() def can_access_page?(nil, _page_path), do: false def can_access_page?(user, page_path) do -- 2.47.2 From d9a5a081dfef39c1b46e0b81dc5e46bbc09ec15a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 11:50:43 +0200 Subject: [PATCH 08/16] refactor(import): drop unreachable CSV error-formatting path consume_and_read_csv/2 and MemberCSV.prepare/2 only ever return {:error, binary()}, so the non-binary error branch and the format_error_message/* helpers it called were unreachable. Removed them and bound the remaining discarded locale/dispatch results. --- lib/mv_web/live/import_live.ex | 128 ++++++++------------------------- 1 file changed, 29 insertions(+), 99 deletions(-) diff --git a/lib/mv_web/live/import_live.ex b/lib/mv_web/live/import_live.ex index f3d4941..a8c5a95 100644 --- a/lib/mv_web/live/import_live.ex +++ b/lib/mv_web/live/import_live.ex @@ -47,14 +47,11 @@ defmodule MvWeb.ImportLive do # after this limit is reached. @max_errors 50 - # Maximum length for error messages before truncation - @max_error_message_length 200 - @impl true def mount(_params, session, socket) do # Get locale from session for translations locale = session["locale"] || "de" - Gettext.put_locale(MvWeb.Gettext, locale) + _ = Gettext.put_locale(MvWeb.Gettext, locale) # Get club name from settings club_name = @@ -193,16 +190,6 @@ defmodule MvWeb.ImportLive do :error, gettext("Failed to prepare CSV import: %{reason}", reason: reason) )} - - {:error, error} -> - error_message = format_error_message(error) - - {:noreply, - put_flash( - socket, - :error, - gettext("Failed to prepare CSV import: %{reason}", reason: error_message) - )} end end @@ -223,64 +210,6 @@ defmodule MvWeb.ImportLive do {:noreply, socket} end - # Formats error messages for user-friendly display. - # - # Handles various error types including Ash errors, maps with message fields, - # lists of errors, and fallback formatting for unknown types. - @spec format_error_message(any()) :: String.t() - defp format_error_message(error) do - case error do - %Ash.Error.Invalid{} = ash_error -> - format_ash_error(ash_error) - - %{message: msg} when is_binary(msg) -> - msg - - %{errors: errors} when is_list(errors) -> - format_error_list(errors) - - reason when is_binary(reason) -> - reason - - other -> - format_unknown_error(other) - end - end - - # Formats Ash validation errors for display - defp format_ash_error(%Ash.Error.Invalid{errors: errors}) when is_list(errors) do - Enum.map_join(errors, ", ", &format_single_error/1) - end - - defp format_ash_error(error) do - format_unknown_error(error) - end - - # Formats a list of errors into a readable string - defp format_error_list(errors) do - Enum.map_join(errors, ", ", &format_single_error/1) - end - - # Formats a single error item - defp format_single_error(error) when is_map(error) do - Map.get(error, :message) || Map.get(error, :field) || inspect(error, limit: :infinity) - end - - defp format_single_error(error) do - to_string(error) - end - - # Formats unknown error types with truncation for very long messages - defp format_unknown_error(other) do - error_str = inspect(other, limit: :infinity, pretty: true) - - if String.length(error_str) > @max_error_message_length do - String.slice(error_str, 0, @max_error_message_length - 3) <> "..." - else - error_str - end - end - @impl true def handle_info({:process_chunk, idx}, socket) do case socket.assigns do @@ -337,32 +266,33 @@ defmodule MvWeb.ImportLive do actor: actor ] - if Config.sql_sandbox?() do - run_chunk_with_locale( - locale, - chunk, - import_state.column_map, - import_state.custom_field_map, - opts, - live_view_pid, - idx - ) - else - Task.Supervisor.start_child( - Mv.TaskSupervisor, - fn -> - run_chunk_with_locale( - locale, - chunk, - import_state.column_map, - import_state.custom_field_map, - opts, - live_view_pid, - idx - ) - end - ) - end + _ = + if Config.sql_sandbox?() do + run_chunk_with_locale( + locale, + chunk, + import_state.column_map, + import_state.custom_field_map, + opts, + live_view_pid, + idx + ) + else + Task.Supervisor.start_child( + Mv.TaskSupervisor, + fn -> + run_chunk_with_locale( + locale, + chunk, + import_state.column_map, + import_state.custom_field_map, + opts, + live_view_pid, + idx + ) + end + ) + end {:noreply, socket} end @@ -378,7 +308,7 @@ defmodule MvWeb.ImportLive do live_view_pid, idx ) do - Gettext.put_locale(MvWeb.Gettext, locale) + _ = Gettext.put_locale(MvWeb.Gettext, locale) ImportRunner.process_chunk(chunk, column_map, custom_field_map, opts, live_view_pid, idx) end -- 2.47.2 From 05f66ccf74a8d28ce9fcde8fca47d68c65ef46ea Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 11:56:44 +0200 Subject: [PATCH 09/16] refactor(types): remove dead catch-all clauses unreachable per success typing --- .../changes/filter_form_data_by_allowlist.ex | 14 ++++---------- lib/membership/member.ex | 2 -- .../changes/set_membership_fee_start_date.ex | 7 ------- lib/mv/config.ex | 7 ------- lib/mv/mailer.ex | 2 -- lib/mv/membership/import/member_csv.ex | 2 -- lib/mv/membership/member_export.ex | 2 -- lib/mv/oidc_role_sync.ex | 2 -- lib/mv_web/controllers/auth_controller.ex | 2 -- .../live/components/member_filter_component.ex | 1 - lib/mv_web/live/group_live/show.ex | 6 ------ lib/mv_web/live/join_live.ex | 2 -- lib/mv_web/live/member_live/index.ex | 10 ---------- .../live/member_live/index/field_selection.ex | 4 ---- priv/gettext/de/LC_MESSAGES/default.po | 10 +++++----- priv/gettext/default.pot | 5 ----- priv/gettext/en/LC_MESSAGES/default.po | 10 +++++----- 17 files changed, 14 insertions(+), 74 deletions(-) diff --git a/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex b/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex index 5de15c8..8dae2d1 100644 --- a/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex +++ b/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex @@ -17,16 +17,10 @@ defmodule Mv.Membership.JoinRequest.Changes.FilterFormDataByAllowlist do form_data = Ash.Changeset.get_attribute(changeset, :form_data) || %{} allowlist_ids = - case Membership.get_join_form_allowlist() do - list when is_list(list) -> - list - |> Enum.map(fn item -> item.id end) - |> MapSet.new() - |> MapSet.difference(MapSet.new(@typed_fields)) - - _ -> - MapSet.new() - end + Membership.get_join_form_allowlist() + |> Enum.map(fn item -> item.id end) + |> MapSet.new() + |> MapSet.difference(MapSet.new(@typed_fields)) filtered = form_data diff --git a/lib/membership/member.ex b/lib/membership/member.ex index bf8a48c..cddc23f 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -1234,8 +1234,6 @@ defmodule Mv.Membership.Member do |> String.replace("_", "\\_") end - defp sanitize_search_query(_), do: "" - # ============================================================================ # Search Filter Builders # ============================================================================ diff --git a/lib/membership_fees/changes/set_membership_fee_start_date.ex b/lib/membership_fees/changes/set_membership_fee_start_date.ex index 0e9cf00..8f5aa56 100644 --- a/lib/membership_fees/changes/set_membership_fee_start_date.ex +++ b/lib/membership_fees/changes/set_membership_fee_start_date.ex @@ -26,8 +26,6 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do """ use Ash.Resource.Change - require Logger - alias Mv.MembershipFees.CalendarCycles @impl true @@ -83,11 +81,6 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do field: :membership_fee_type_id, message: "not found" ) - - {:error, reason} -> - # Log warning for other unexpected errors - Logger.warning("Could not auto-set membership_fee_start_date: #{inspect(reason)}") - changeset end end diff --git a/lib/mv/config.ex b/lib/mv/config.ex index f198191..750a7db 100644 --- a/lib/mv/config.ex +++ b/lib/mv/config.ex @@ -207,8 +207,6 @@ defmodule Mv.Config do end end - defp derive_app_url_from_api_url(_), do: nil - @doc """ Returns true if Vereinfacht is fully configured (URL, API key, and club ID all set). """ @@ -251,7 +249,6 @@ defmodule Mv.Config do case System.get_env(key) do nil -> false v when is_binary(v) -> String.trim(v) != "" - _ -> false end end @@ -270,9 +267,6 @@ defmodule Mv.Config do value when is_binary(value) -> v = String.trim(value) |> String.downcase() v in ["true", "1", "yes"] - - _ -> - false end end @@ -328,7 +322,6 @@ defmodule Mv.Config do defp present?(nil), do: false defp present?(s) when is_binary(s), do: String.trim(s) != "" - defp present?(_), do: false # --------------------------------------------------------------------------- # OIDC authentication diff --git a/lib/mv/mailer.ex b/lib/mv/mailer.ex index ec8f357..1e55b6e 100644 --- a/lib/mv/mailer.ex +++ b/lib/mv/mailer.ex @@ -190,6 +190,4 @@ defmodule Mv.Mailer do defp valid_email?(email) when is_binary(email) do Regex.match?(@email_regex, String.trim(email)) end - - defp valid_email?(_), do: false end diff --git a/lib/mv/membership/import/member_csv.ex b/lib/mv/membership/import/member_csv.ex index 23e0d93..dda1d04 100644 --- a/lib/mv/membership/import/member_csv.ex +++ b/lib/mv/membership/import/member_csv.ex @@ -210,8 +210,6 @@ defmodule Mv.Membership.Import.MemberCSV do MapSet.member?(HeaderMapper.known_member_fields(), normalized) end - defp member_field?(_), do: false - # Validates that row count doesn't exceed limit defp validate_row_count(rows, max_rows) do if length(rows) > max_rows do diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex index d96c82f..81cc164 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -522,6 +522,4 @@ defmodule Mv.Membership.MemberExport do other -> other end) end - - defp normalize_computed_fields(_), do: [] end diff --git a/lib/mv/oidc_role_sync.ex b/lib/mv/oidc_role_sync.ex index a13748a..0f6467c 100644 --- a/lib/mv/oidc_role_sync.ex +++ b/lib/mv/oidc_role_sync.ex @@ -87,8 +87,6 @@ defmodule Mv.OidcRoleSync do ArgumentError -> nil end - defp safe_get_atom(_map, _key), do: nil - defp peek_jwt_claims(token) do parts = String.split(token, ".") diff --git a/lib/mv_web/controllers/auth_controller.ex b/lib/mv_web/controllers/auth_controller.ex index 120c245..42bdcfa 100644 --- a/lib/mv_web/controllers/auth_controller.ex +++ b/lib/mv_web/controllers/auth_controller.ex @@ -335,8 +335,6 @@ defmodule MvWeb.AuthController do end end - defp redact_url(_), do: "[redacted]" - def sign_out(conn, _params) do conn = clear_session(conn, :mv) |> put_flash(:success, gettext("You are now signed out")) diff --git a/lib/mv_web/live/components/member_filter_component.ex b/lib/mv_web/live/components/member_filter_component.ex index 99ee2c5..b66d259 100644 --- a/lib/mv_web/live/components/member_filter_component.ex +++ b/lib/mv_web/live/components/member_filter_component.ex @@ -935,7 +935,6 @@ defmodule MvWeb.Components.MemberFilterComponent do {nil, true} -> "#{base_classes} btn-active" {:in, true} -> "#{base_classes} btn-success btn-active" {:not_in, true} -> "#{base_classes} btn-error btn-active" - _ -> "#{base_classes} btn-outline" end end diff --git a/lib/mv_web/live/group_live/show.ex b/lib/mv_web/live/group_live/show.ex index b9f22c8..7cd4378 100644 --- a/lib/mv_web/live/group_live/show.ex +++ b/lib/mv_web/live/group_live/show.ex @@ -836,12 +836,6 @@ defmodule MvWeb.GroupLive.Show do end end - defp perform_add_members(socket, _group, _member_ids, _actor) do - {:noreply, - socket - |> put_flash(:error, gettext("No members selected."))} - end - defp handle_successful_add_members(socket, group, actor) do socket = reload_group(socket, group.slug, actor) diff --git a/lib/mv_web/live/join_live.ex b/lib/mv_web/live/join_live.ex index ba0e476..b679127 100644 --- a/lib/mv_web/live/join_live.ex +++ b/lib/mv_web/live/join_live.ex @@ -287,8 +287,6 @@ defmodule MvWeb.JoinLive do end end - defp member_field_input_type(_), do: "text" - defp member_field_atom(field_id) when is_binary(field_id) do Mv.Constants.member_fields() |> Enum.find(&(Atom.to_string(&1) == field_id)) diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index cd32513..6196fc4 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -1218,8 +1218,6 @@ defmodule MvWeb.MemberLive.Index do end end - defp apply_one_fee_type_filter(query, _, _), do: query - defp apply_cycle_status_filter(members, nil, _show_current), do: members defp apply_cycle_status_filter(members, status, show_current) @@ -1297,8 +1295,6 @@ defmodule MvWeb.MemberLive.Index do end end - defp valid_sort_field?(_), do: false - defp valid_sort_field_db_or_custom?(field) when is_atom(field) do non_sortable_fields = [:notes] valid_fields = Mv.Constants.member_fields() -- non_sortable_fields @@ -1558,8 +1554,6 @@ defmodule MvWeb.MemberLive.Index do assign(socket, :group_filters, Map.take(filters, valid_group_ids)) end - defp maybe_update_group_filters(socket, _), do: socket - defp maybe_update_fee_type_filters(socket, params) when is_map(params) do prefix = @fee_type_filter_prefix prefix_len = String.length(prefix) @@ -1586,8 +1580,6 @@ defmodule MvWeb.MemberLive.Index do assign(socket, :fee_type_filters, Map.take(filters, valid_fee_type_ids)) end - defp maybe_update_fee_type_filters(socket, _), do: socket - defp add_fee_type_filter_entry(acc, key, value_str, prefix_len) do key_str = to_string(key) raw_id = String.slice(key_str, prefix_len, String.length(key_str) - prefix_len) @@ -1719,8 +1711,6 @@ defmodule MvWeb.MemberLive.Index do assign(socket, :date_filters, DateFilter.from_params(params, date_custom_fields)) end - defp maybe_update_date_filters(socket, _params), do: socket - # ------------------------------------------------------------- # Custom Field Value Helpers # ------------------------------------------------------------- diff --git a/lib/mv_web/live/member_live/index/field_selection.ex b/lib/mv_web/live/member_live/index/field_selection.ex index 1e77b09..846cf1d 100644 --- a/lib/mv_web/live/member_live/index/field_selection.ex +++ b/lib/mv_web/live/member_live/index/field_selection.ex @@ -103,8 +103,6 @@ defmodule MvWeb.MemberLive.Index.FieldSelection do end) end - defp parse_cookie_header(_), do: %{} - @doc """ Saves field selection to cookie. @@ -218,8 +216,6 @@ defmodule MvWeb.MemberLive.Index.FieldSelection do end end - defp parse_json(_), do: %{} - # Parses a comma-separated string of field names defp parse_fields_string(fields_string) do fields_string diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index f03daf0..81d91f7 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -2208,11 +2208,6 @@ msgstr "Keine Mitglieder in dieser Gruppe" msgid "No members selected" msgstr "Keine Mitglieder ausgewählt" -#: lib/mv_web/live/group_live/show.ex -#, elixir-autogen, elixir-format -msgid "No members selected." -msgstr "Keine Mitglieder ausgewählt." - #: lib/mv_web/live/member_live/show/membership_fees_component.ex #, elixir-autogen, elixir-format msgid "No membership fee cycles found. Cycles will be generated automatically when a membership fee type is assigned." @@ -3972,3 +3967,8 @@ msgstr "Zeitraum" #, elixir-autogen, elixir-format msgid "To" msgstr "Bis" + +#~ #: lib/mv_web/live/group_live/show.ex +#~ #, elixir-autogen, elixir-format +#~ msgid "No members selected." +#~ msgstr "Keine Mitglieder ausgewählt." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 50ceff8..5e9abca 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2209,11 +2209,6 @@ msgstr "" msgid "No members selected" msgstr "" -#: lib/mv_web/live/group_live/show.ex -#, elixir-autogen, elixir-format -msgid "No members selected." -msgstr "" - #: lib/mv_web/live/member_live/show/membership_fees_component.ex #, elixir-autogen, elixir-format msgid "No membership fee cycles found. Cycles will be generated automatically when a membership fee type is assigned." diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 9ec230f..1ae6a49 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2209,11 +2209,6 @@ msgstr "" msgid "No members selected" msgstr "" -#: lib/mv_web/live/group_live/show.ex -#, elixir-autogen, elixir-format, fuzzy -msgid "No members selected." -msgstr "" - #: lib/mv_web/live/member_live/show/membership_fees_component.ex #, elixir-autogen, elixir-format msgid "No membership fee cycles found. Cycles will be generated automatically when a membership fee type is assigned." @@ -3972,3 +3967,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "To" msgstr "" + +#~ #: lib/mv_web/live/group_live/show.ex +#~ #, elixir-autogen, elixir-format, fuzzy +#~ msgid "No members selected." +#~ msgstr "" -- 2.47.2 From c41d24113feb8c653fa165eff687000b38a14d06 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 12:00:38 +0200 Subject: [PATCH 10/16] fix(import): return readable string for unreadable upload errors File.read/1 only yields posix atoms, so the File.Error and bare-reason branches were unreachable, and :file.format_error/1 returns a charlist rather than a String. Normalize the error to a binary so it interpolates correctly in flash messages. --- lib/mv/membership/import/import_runner.ex | 8 +---- .../membership/import/import_runner_test.exs | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 test/mv/membership/import/import_runner_test.exs diff --git a/lib/mv/membership/import/import_runner.ex b/lib/mv/membership/import/import_runner.ex index eccd75f..5f953d4 100644 --- a/lib/mv/membership/import/import_runner.ex +++ b/lib/mv/membership/import/import_runner.ex @@ -26,14 +26,8 @@ defmodule Mv.Membership.Import.ImportRunner do {:ok, content} -> {:ok, content} - {:error, reason} when is_atom(reason) -> - {:error, :file.format_error(reason)} - - {:error, %File.Error{reason: reason}} -> - {:error, :file.format_error(reason)} - {:error, reason} -> - {:error, Exception.message(reason)} + {:error, to_string(:file.format_error(reason))} end end diff --git a/test/mv/membership/import/import_runner_test.exs b/test/mv/membership/import/import_runner_test.exs new file mode 100644 index 0000000..88d189e --- /dev/null +++ b/test/mv/membership/import/import_runner_test.exs @@ -0,0 +1,33 @@ +defmodule Mv.Membership.Import.ImportRunnerTest do + use ExUnit.Case, async: true + + alias Mv.Membership.Import.ImportRunner + + describe "read_file_entry/2" do + test "returns {:ok, content} for a readable file" do + path = + Path.join( + System.tmp_dir!(), + "import_runner_read_#{System.unique_integer([:positive])}.csv" + ) + + File.write!(path, "email;first_name\njohn@example.com;John") + on_exit(fn -> File.rm_rf(path) end) + + assert {:ok, "email;first_name\njohn@example.com;John"} = + ImportRunner.read_file_entry(%{path: path}, %{}) + end + + test "returns {:error, message} with a binary message when the file cannot be read" do + missing_path = + Path.join( + System.tmp_dir!(), + "import_runner_missing_#{System.unique_integer([:positive])}.csv" + ) + + assert {:error, message} = ImportRunner.read_file_entry(%{path: missing_path}, %{}) + assert is_binary(message) + assert message != "" + end + end +end -- 2.47.2 From 2db467d5d16d76f574a8edb7ec6c2ba4a89378ff Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 12:04:37 +0200 Subject: [PATCH 11/16] fix(pdf-export): match DateTime.from_iso8601 three-tuple when formatting cells DateTime.from_iso8601/1 returns {:ok, datetime, offset}, so the two-tuple clauses never matched and datetime cells fell through to the naive-parse fallback. Matching the real shape routes them through the intended DateTime path; UTC values render identically. --- lib/mv/membership/members_pdf.ex | 9 ++------- test/mv/membership/members_pdf_test.exs | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lib/mv/membership/members_pdf.ex b/lib/mv/membership/members_pdf.ex index b2989ca..a1c8418 100644 --- a/lib/mv/membership/members_pdf.ex +++ b/lib/mv/membership/members_pdf.ex @@ -143,7 +143,7 @@ defmodule Mv.Membership.MembersPDF do defp convert_to_template_format(export_data, locale, club_name) do # Set locale for translations - Gettext.put_locale(MvWeb.Gettext, locale) + _ = Gettext.put_locale(MvWeb.Gettext, locale) headers = Enum.map(export_data.columns, & &1.label) column_count = length(export_data.columns) @@ -211,9 +211,6 @@ defmodule Mv.Membership.MembersPDF do {:ok, datetime, _offset} -> format_datetime(datetime, locale) - {:ok, datetime} -> - format_datetime(datetime, locale) - {:error, _} -> # Try NaiveDateTime if DateTime parsing fails case NaiveDateTime.from_iso8601(iso8601_string) do @@ -257,8 +254,6 @@ defmodule Mv.Membership.MembersPDF do end end - defp format_date(_, _), do: "" - defp format_dates_in_rows(rows, columns, locale) do date_indices = find_date_column_indices(columns) @@ -321,7 +316,7 @@ defmodule Mv.Membership.MembersPDF do defp format_cell_date_datetime(cell_value, locale) do case DateTime.from_iso8601(cell_value) do - {:ok, datetime} -> format_datetime(datetime, locale) + {:ok, datetime, _offset} -> format_datetime(datetime, locale) _ -> format_cell_date_naive(cell_value, locale) end end diff --git a/test/mv/membership/members_pdf_test.exs b/test/mv/membership/members_pdf_test.exs index 78a8ca6..2b83e3b 100644 --- a/test/mv/membership/members_pdf_test.exs +++ b/test/mv/membership/members_pdf_test.exs @@ -101,6 +101,29 @@ defmodule Mv.Membership.MembersPDFTest do assert byte_size(pdf_binary) > 1000 end + test "renders date column holding an ISO8601 datetime value" do + # Regression: a date column whose value is a full datetime string must be + # parsed via DateTime.from_iso8601/1 (which returns a 3-tuple) and rendered, + # not silently dropped. + export_data = %{ + columns: [ + %{key: "first_name", kind: :member_field, label: "Vorname"}, + %{key: "join_date", kind: :member_field, label: "Eintritt"} + ], + rows: [ + ["Max", "2024-01-15T14:30:00Z"] + ], + meta: %{ + generated_at: "2024-01-15T14:30:00Z", + member_count: 1 + } + } + + assert {:ok, pdf_binary} = MembersPDF.render(export_data) + assert String.starts_with?(pdf_binary, "%PDF") + assert byte_size(pdf_binary) > 1000 + end + test "generates valid PDF with custom fields and computed fields" do export_data = %{ columns: [ -- 2.47.2 From ec6422d450ccb2f62d447861a4ed790e99c6d05e Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 12:08:19 +0200 Subject: [PATCH 12/16] fix(membership-fees): show error for unparseable cycle date instead of crashing Date.from_iso8601/1 returns {:error, reason}, so the with else clause matching a bare :error never fired and an invalid date raised a WithClauseError. Match the real date/calendar error reasons so the user sees the validation message. --- .../show/membership_fees_component.ex | 2 +- .../member_live/show_membership_fees_test.exs | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/mv_web/live/member_live/show/membership_fees_component.ex b/lib/mv_web/live/member_live/show/membership_fees_component.ex index e8ddff4..0cba316 100644 --- a/lib/mv_web/live/member_live/show/membership_fees_component.ex +++ b/lib/mv_web/live/member_live/show/membership_fees_component.ex @@ -1027,7 +1027,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do |> assign(:create_cycle_error, format_error(error))} end else - :error -> + {:error, reason} when reason in [:invalid_format, :invalid_date, :incompatible_calendars] -> {:noreply, socket |> assign(:create_cycle_error, gettext("Invalid date format"))} diff --git a/test/mv_web/member_live/show_membership_fees_test.exs b/test/mv_web/member_live/show_membership_fees_test.exs index 2abb0cb..59dc471 100644 --- a/test/mv_web/member_live/show_membership_fees_test.exs +++ b/test/mv_web/member_live/show_membership_fees_test.exs @@ -268,6 +268,28 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do # Should not crash assert html =~ member.first_name end + + test "create_cycle with an unparseable date shows an error instead of crashing", %{conn: conn} do + fee_type = create_fee_type(%{interval: :yearly}) + member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) + + {:ok, view, _html} = live(conn, "/members/#{member.id}") + + view + |> element("button[phx-click='switch_tab'][phx-value-tab='membership_fees']") + |> render_click() + + view + |> element("button[phx-click='open_create_cycle_modal']") + |> render_click() + + html = + view + |> element("form[phx-submit='create_cycle']") + |> render_submit(%{"date" => "not-a-date", "amount" => "10"}) + + assert html =~ "Invalid date format" + end end describe "read_only user (Vorstand/Buchhaltung) - no cycle action buttons" do -- 2.47.2 From 6a4a99f638720823fad490cf9b6c89b7ad809b79 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 12:11:59 +0200 Subject: [PATCH 13/16] refactor(types): drop guards and clauses that can never succeed --- .../membership/member/validations/email_change_permission.ex | 2 +- lib/mv/membership/member_export.ex | 3 --- lib/mv/oidc_role_sync_config.ex | 2 +- lib/mv_web/live/membership_fee_settings_live.ex | 1 - lib/mv_web/live/membership_fee_type_live/index.ex | 1 - lib/mv_web/live/user_live/form.ex | 3 +-- 6 files changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/mv/membership/member/validations/email_change_permission.ex b/lib/mv/membership/member/validations/email_change_permission.ex index 2b1c041..073da07 100644 --- a/lib/mv/membership/member/validations/email_change_permission.ex +++ b/lib/mv/membership/member/validations/email_change_permission.ex @@ -59,7 +59,7 @@ defmodule Mv.Membership.Member.Validations.EmailChangePermission do # Ash stores actor in changeset.context.private.actor; validation context has .actor; some callsites use context.actor defp resolve_actor(changeset, context) do - ctx = changeset.context || %{} + ctx = changeset.context get_in(ctx, [:private, :actor]) || Map.get(ctx, :actor) || diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex index 81cc164..a98b125 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -473,9 +473,6 @@ defmodule Mv.Membership.MemberExport do computed_fields, member_fields ) do - computed_fields = computed_fields || [] - member_fields = member_fields || [] - db_with_insert = Enum.flat_map(db_fields_ordered, fn f -> expand_field_with_computed(f, member_fields, computed_fields) diff --git a/lib/mv/oidc_role_sync_config.ex b/lib/mv/oidc_role_sync_config.ex index 2a8574c..bbb5770 100644 --- a/lib/mv/oidc_role_sync_config.ex +++ b/lib/mv/oidc_role_sync_config.ex @@ -15,6 +15,6 @@ defmodule Mv.OidcRoleSyncConfig do @doc "Returns the JWT/user_info claim name for groups; defaults to \"groups\"." def oidc_groups_claim do - Mv.Config.oidc_groups_claim() || "groups" + Mv.Config.oidc_groups_claim() end end diff --git a/lib/mv_web/live/membership_fee_settings_live.ex b/lib/mv_web/live/membership_fee_settings_live.ex index e153d18..15030c1 100644 --- a/lib/mv_web/live/membership_fee_settings_live.ex +++ b/lib/mv_web/live/membership_fee_settings_live.ex @@ -464,7 +464,6 @@ defmodule MvWeb.MembershipFeeSettingsLive do Enum.map_join(error.errors, ", ", fn e -> e.message end) end - defp format_error(error) when is_binary(error), do: error defp format_error(_error), do: gettext("An error occurred") defp assign_form(%{assigns: %{settings: settings}} = socket) do diff --git a/lib/mv_web/live/membership_fee_type_live/index.ex b/lib/mv_web/live/membership_fee_type_live/index.ex index a34480b..65f840d 100644 --- a/lib/mv_web/live/membership_fee_type_live/index.ex +++ b/lib/mv_web/live/membership_fee_type_live/index.ex @@ -214,7 +214,6 @@ defmodule MvWeb.MembershipFeeTypeLive.Index do Enum.map_join(error.errors, ", ", fn e -> e.message end) end - defp format_error(error) when is_binary(error), do: error defp format_error(_error), do: gettext("An error occurred") # Info card explaining the membership fee type concept diff --git a/lib/mv_web/live/user_live/form.ex b/lib/mv_web/live/user_live/form.ex index 622228d..60763ab 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -922,7 +922,7 @@ defmodule MvWeb.UserLive.Form do end # Extract user-friendly error message from Ash.Error - @spec extract_error_message(any()) :: String.t() + @spec extract_error_message(Ash.Error.t()) :: String.t() defp extract_error_message(%Ash.Error.Invalid{errors: errors}) when is_list(errors) do # Take first error and extract message case List.first(errors) do @@ -932,6 +932,5 @@ defmodule MvWeb.UserLive.Form do end end - defp extract_error_message(error) when is_binary(error), do: error defp extract_error_message(_), do: gettext("Unknown error") end -- 2.47.2 From a7ad60805138cfc64ea1b548d7514ff11d2964ec Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 12:19:21 +0200 Subject: [PATCH 14/16] fix(auth): redirect a live-view socket in the user-required guard LiveSession.assign_new_resources/2 is typed to return a Phoenix.Socket, which made the on_mount redirect type-incompatible. The authenticated-routes live_session already assigns current_user, so the guard reads it from socket.assigns directly. Also assign the locale into the socket actually used by the no-user redirect instead of discarding it. --- lib/mv_web/live_user_auth.ex | 13 +++++------ test/mv_web/live_user_auth_test.exs | 35 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 test/mv_web/live_user_auth_test.exs diff --git a/lib/mv_web/live_user_auth.ex b/lib/mv_web/live_user_auth.ex index f3f3fc9..617b079 100644 --- a/lib/mv_web/live_user_auth.ex +++ b/lib/mv_web/live_user_auth.ex @@ -31,27 +31,24 @@ defmodule MvWeb.LiveUserAuth do end end - def on_mount(:live_user_required, _params, session, socket) do - socket = LiveSession.assign_new_resources(socket, session) - + def on_mount(:live_user_required, _params, _session, socket) do case socket.assigns do %{current_user: %{} = user} -> {:cont, assign(socket, :current_user, user)} _ -> - socket = LiveView.redirect(socket, to: ~p"/sign-in") - {:halt, socket} + {:halt, LiveView.redirect(socket, to: ~p"/sign-in")} end end def on_mount(:live_no_user, _params, session, socket) do # Set the locale for not logged in user (default from config, "de" in dev/prod). locale = session["locale"] || Application.get_env(:mv, :default_locale, "de") - Gettext.put_locale(MvWeb.Gettext, locale) - {:cont, assign(socket, :locale, locale)} + _ = Gettext.put_locale(MvWeb.Gettext, locale) + socket = assign(socket, :locale, locale) if socket.assigns[:current_user] do - {:halt, Phoenix.LiveView.redirect(socket, to: ~p"/")} + {:halt, LiveView.redirect(socket, to: ~p"/")} else {:cont, assign(socket, :current_user, nil)} end diff --git a/test/mv_web/live_user_auth_test.exs b/test/mv_web/live_user_auth_test.exs new file mode 100644 index 0000000..0f0e1ae --- /dev/null +++ b/test/mv_web/live_user_auth_test.exs @@ -0,0 +1,35 @@ +defmodule MvWeb.LiveUserAuthTest do + @moduledoc """ + Regression tests for the `MvWeb.LiveUserAuth` on_mount guards: + the unauthenticated `:live_user_required` redirect to the sign-in page and + the authenticated `:live_no_user` redirect away from the sign-in page. + """ + use MvWeb.ConnCase, async: false + + import Phoenix.LiveViewTest + + describe ":live_user_required" do + @tag role: :unauthenticated + test "unauthenticated request to a protected route is redirected to sign-in", %{conn: conn} do + assert {:error, {:redirect, %{to: to}}} = live(conn, "/members") + assert to == "/sign-in" + end + + @tag role: :admin + test "authenticated user can mount a protected route", %{conn: conn} do + assert {:ok, _view, _html} = live(conn, "/members") + end + end + + describe ":live_no_user" do + @tag role: :admin + test "authenticated user visiting the sign-in page is redirected to root", %{conn: conn} do + assert {:error, {:redirect, %{to: "/"}}} = live(conn, "/sign-in") + end + + @tag role: :unauthenticated + test "unauthenticated user can reach the sign-in page", %{conn: conn} do + assert {:ok, _view, _html} = live(conn, "/sign-in") + end + end +end -- 2.47.2 From b5756d8e00a5e1dda9b1e4abe1db15f408c2a72e Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 12:23:04 +0200 Subject: [PATCH 15/16] refactor(vereinfacht): gate retry skipping on runtime sandbox flag The compile-time Mix.env() comparison folded to an always-false literal under analysis. sql_sandbox?/0 reads runtime config (true only in test) and works in releases where Mix is unavailable, preserving the fast-fail-no-retry behavior in tests. --- lib/mv/vereinfacht/client.ex | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/mv/vereinfacht/client.ex b/lib/mv/vereinfacht/client.ex index 6a81c46..999bd44 100644 --- a/lib/mv/vereinfacht/client.ex +++ b/lib/mv/vereinfacht/client.ex @@ -98,13 +98,12 @@ defmodule Mv.Vereinfacht.Client do @sync_timeout_ms 5_000 - # Resolved at compile time so Mix is never called at runtime (Mix is not available in releases). - @env Mix.env() - # In test, skip retries so sync fails fast when no API is running (avoids log spam and long waits). + # `sql_sandbox?/0` reads runtime config (true only in test) and avoids calling Mix at runtime, + # which is unavailable in releases. defp req_http_options do opts = [receive_timeout: @sync_timeout_ms] - if @env == :test, do: [retry: false] ++ opts, else: opts + if Mv.Config.sql_sandbox?(), do: [retry: false] ++ opts, else: opts end defp post_and_parse_contact(url, body, api_key) do -- 2.47.2 From 9a14cedc149ce8d7243a109c956b3de6eb087415 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 2 Jun 2026 12:26:35 +0200 Subject: [PATCH 16/16] fix(repo): define all_tenants/0 as empty for non-multitenant schema --- lib/mv/repo.ex | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/mv/repo.ex b/lib/mv/repo.ex index 0a4a04d..183c54f 100644 --- a/lib/mv/repo.ex +++ b/lib/mv/repo.ex @@ -19,4 +19,12 @@ defmodule Mv.Repo do def min_pg_version do %Version{major: 17, minor: 2, patch: 0} end + + # This app does not use schema-based multitenancy, so there are no tenant + # schemas to migrate. Returning [] keeps the AshPostgres callback total + # rather than raising the default "not defined" error. + @impl true + def all_tenants do + [] + end end -- 2.47.2