diff --git a/.deps_audit_ignore b/.deps_audit_ignore deleted file mode 100644 index 27c623d..0000000 --- a/.deps_audit_ignore +++ /dev/null @@ -1,9 +0,0 @@ -# Temporarily ignored security advisories -# -# Format: one GHSA ID per line. -# Remove an entry once a patched version is available and the dependency is updated. - -# cowlib >= 2.9.0 <= 2.16.1 — Cookie Request Header Injection via cow_cookie:cookie/1 -# Severity: low. No patched version available as of 2026-05-20. -# Tracked upstream: https://github.com/advisories/GHSA-g2wm-735q-3f56 -GHSA-g2wm-735q-3f56 diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs deleted file mode 100644 index c89978c..0000000 --- a/.dialyzer_ignore.exs +++ /dev/null @@ -1,11 +0,0 @@ -# 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 deleted file mode 100644 index 388e8f4..0000000 --- a/.drone.jsonnet +++ /dev/null @@ -1,184 +0,0 @@ -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', './priv/plts'], 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_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, - 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', './priv/plts'] }, - 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, - ] + (if test.name == 'test-all' then [step_typecheck] else []) + [ - 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 new file mode 100644 index 0000000..b0fb160 --- /dev/null +++ b/.drone.yml @@ -0,0 +1,298 @@ +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 + # 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 + # 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 diff --git a/.env.example b/.env.example index bc0ef7a..d63e019 100644 --- a/.env.example +++ b/.env.example @@ -24,7 +24,7 @@ ASSOCIATION_NAME="Sportsclub XYZ" # OIDC_CLIENT_ID=mv # OIDC_BASE_URL=http://localhost:8080/auth/v1 # OIDC_REDIRECT_URI=http://localhost:4001/auth/user/oidc/callback -# OIDC_CLIENT_SECRET=mv-dev-shared-secret-not-for-production-do-not-use-anywhere-else +# OIDC_CLIENT_SECRET=your-oidc-client-secret # Optional: OIDC group → Admin role sync (e.g. Authentik groups from profile scope) # If OIDC_ADMIN_GROUP_NAME is set, users in that group get Admin role on registration/sign-in. diff --git a/.gitignore b/.gitignore index b37fa85..058543c 100644 --- a/.gitignore +++ b/.gitignore @@ -34,7 +34,6 @@ mv-*.tar # In case you use Node.js/npm, you want to ignore these. npm-debug.log /assets/node_modules/ -/node_modules/ .cursor @@ -46,11 +45,3 @@ npm-debug.log # Docker secrets directory (generated by `just init-secrets`) /secrets/ 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/.opencode/screenshots/01_mitglieder.png b/.opencode/screenshots/01_mitglieder.png deleted file mode 100644 index 7cf25af..0000000 Binary files a/.opencode/screenshots/01_mitglieder.png and /dev/null differ diff --git a/.opencode/screenshots/02_statistik.png b/.opencode/screenshots/02_statistik.png deleted file mode 100644 index 675c036..0000000 Binary files a/.opencode/screenshots/02_statistik.png and /dev/null differ diff --git a/.opencode/screenshots/03_beitraege.png b/.opencode/screenshots/03_beitraege.png deleted file mode 100644 index 5918953..0000000 Binary files a/.opencode/screenshots/03_beitraege.png and /dev/null differ diff --git a/.opencode/screenshots/04_aufnahmeantraege.png b/.opencode/screenshots/04_aufnahmeantraege.png deleted file mode 100644 index 13bb316..0000000 Binary files a/.opencode/screenshots/04_aufnahmeantraege.png and /dev/null differ diff --git a/.tool-versions b/.tool-versions index e815bde..e72ed5f 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,4 +1,3 @@ elixir 1.18.3-otp-27 erlang 27.3.4 -just 1.51.0 -nodejs 26.2.0 +just 1.50.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index adbe7e7..8c8032c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,26 +5,6 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] - -### Added -- **GDPR/DSGVO join-form description** – Custom fields can carry a "join form description" that is shown as the field's label on the public join form, with clickable external links (whole URLs and Markdown `[text](url)`). Useful for presenting a GDPR confirmation with a link to an externally hosted privacy declaration before sign-up. -- **Join-form description tooltip in member details** – Custom fields that have a join-form description show an info tooltip (prefixed "Beitrittsformular:") on their label in the member detail view. -- **Editable join-form description** – Admins can set a field's join-form description in the custom-field settings, with an inline hint about the supported link syntax. -- **CSV import – groups column** – Members can be assigned to groups during CSV import via a `Groups`/`Gruppen` column; group names that do not exist yet are created automatically, and re-importing the same file does not create duplicate groups. -- **CSV import – membership fee type column** – A `Fee Type`/`Beitragsart` column assigns each member's membership fee type; an unknown name falls back to the default fee type and is flagged in the preview with a link to create it. -- **CSV import – mapping preview** – After uploading a file, a preview shows how every column maps (with sample rows and warnings for ignored or unknown columns) and the import only starts once you confirm. -- **Dynamic CSV import templates** – The EN and DE import-template downloads now include the association's current custom fields instead of a fixed column set. - -### Changed -- **Member bulk actions in one menu** – The actions above the member overview (open in email program, copy email addresses, export to CSV, export to PDF) are now collected in a single "Aktionen" dropdown instead of separate buttons. Without a selection they apply to all members, or to the currently filtered members; the trigger shows the active scope. Opening the email program is disabled when too many recipients are selected, with a hint to copy the addresses or use the export instead. -- **Dropdown buttons** – Dropdown buttons (actions, filter, column visibility) now show a chevron so they are recognizable as menus. -- **Default GDPR custom field** – The seeded GDPR field was shortened from "Datenschutzerklärung akzeptiert" to "DSGVO" and now ships with a default join-form description (with a placeholder link to replace). - -### Fixed -- **CSV date round-trip** – Date custom-field values are now exported as ISO-8601 (`YYYY-MM-DD`), so an exported CSV can be re-imported without date-parsing errors. -- **CSV import – fee-status columns ignored** – Columns such as `Bezahlstatus` / `Membership Fee Status` are always ignored on import and never stored as a custom-field value, even when a custom field of the same name exists. - ## [1.2.0] - 2026-05-08 ### Changed diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index ccd16f4..2b378ef 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -1363,8 +1363,6 @@ mix gettext.merge priv/gettext --on-obsolete=mark_as_obsolete ### 3.13 Task Runner: Just -The `Justfile` prepends `~/.asdf/shims`, `~/.asdf/bin`, and `~/.asdf` to `PATH` for all recipes (`set export := true`), so `mix` / `elixir` resolve from `.tool-versions` without shell init. The caller's `PATH` is kept (e.g. Homebrew `asdf`, Docker). Run `asdf install` once per machine; no extra `source` is required for `just run`. - **Common Commands:** ```bash diff --git a/Justfile b/Justfile index 9b0be65..d2c51e5 100644 --- a/Justfile +++ b/Justfile @@ -1,12 +1,6 @@ set dotenv-load := true set export := true -# Prepend asdf paths so recipes work without sourcing ~/.asdf/asdf.sh in the shell. -# Caller PATH is preserved (Homebrew asdf, docker CLI, etc.). See CODE_GUIDELINES §3.13. -home := env_var("HOME") -asdf_paths := home + "/.asdf/shims:" + home + "/.asdf/bin:" + home + "/.asdf:" -PATH := asdf_paths + env_var("PATH") - MIX_QUIET := "1" run: install-dependencies start-database migrate-database seed-database @@ -29,27 +23,7 @@ seed-database: start-database: docker compose up -d -# Full check suite: lint + audit + the fast tests (slow/ui excluded). No Dialyzer. -ci-dev: install-dependencies lint audit test-fast - -# Fast pre-commit check: lint + sobelow + only the affected tests (mix test --stale) -# with reduced property runs. Run the full `ci-dev` before pushing. -check: install-dependencies lint sobelow test-stale - -# 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 +ci-dev: lint audit test-fast gettext: mix gettext.extract @@ -63,28 +37,19 @@ lint: @bash -c 'for file in priv/gettext/de/LC_MESSAGES/*.po; do awk "/^msgid \"\"$/{header=1; next} /^msgid /{header=0} /^msgstr \"\"$/ && !header{print FILENAME\":\"NR\": \" \$0; exit 1}" "$file" || exit 1; done' mix gettext.extract --check-up-to-date -# Static security scan (Sobelow). -sobelow: +audit: mix sobelow --config - -# Full security audit: Sobelow + dependency advisory scans. -audit: sobelow - mix deps.audit --ignore-file .deps_audit_ignore + mix deps.audit mix hex.audit -# Run all tests. No install-dependencies prerequisite so single-file runs stay -# fast; run `just install-dependencies` once on a fresh checkout. -test *args: +# Run all tests +test *args: install-dependencies mix test {{args}} -# Fast tests only (excludes slow/performance and UI tests). -test-fast *args: +# Run only fast tests (excludes slow/performance and UI tests) +test-fast *args: install-dependencies mix test --exclude slow --exclude ui {{args}} -# Affected fast tests only (mix test --stale) with reduced property runs. -test-stale *args: - PROPERTY_RUNS=25 mix test --stale --exclude slow --exclude ui {{args}} - # Run only UI tests ui *args: install-dependencies mix test --only ui {{args}} @@ -104,10 +69,6 @@ test-all *args: install-dependencies format: mix format -# Catch-all wrapper for arbitrary mix commands not exposed as their own recipe. -mix *args: - mix {{args}} - build-docker-container: docker build --tag mitgliederverwaltung . diff --git a/README.md b/README.md index 8b26327..9fc2f83 100644 --- a/README.md +++ b/README.md @@ -124,8 +124,8 @@ mix archive.install hex phx_new 1. Copy env file: ```bash cp .env.example .env + # Set OIDC_CLIENT_SECRET inside .env ``` - The dev `OIDC_CLIENT_SECRET` is already preset — no manual GUI step needed. 2. Start everything (database, Mailcrab, Rauthy, app): ```bash @@ -139,9 +139,21 @@ mix archive.install hex phx_new ## 🔐 Testing SSO locally -A local **Rauthy** instance is provided in dev. The `mv` client is auto-seeded from `rauthy-bootstrap/clients.json` on first start (and after `docker compose down -v`), so the secret in `.env.example` always matches. +Mila uses OIDC for Single Sign-On. In development, a local **Rauthy** instance is provided. -Rauthy admin UI: — login `admin@localhost`, password from `BOOTSTRAP_ADMIN_PASSWORD_PLAIN` in `docker-compose.yml`. +1. `just run` +2. go to [localhost:8080](http://localhost:8080), go to the Admin area +3. Login with "admin@localhost" and password from `BOOTSTRAP_ADMIN_PASSWORD_PLAIN` in docker-compose.yml +4. add client from the admin panel + - Client ID: mv + - redirect uris: http://localhost:4000/auth/user/oidc/callback + - Authorization Flows: authorization_code + - allowed origins: http://localhost:4000 + - access/id token algortihm: RS256 (EDDSA did not work for me, found just few infos in the ashauthentication docs) +5. copy client secret to `.env` file +6. abort and run `just run` again + +Now you can log in to Mila via OIDC! ### OIDC with other providers (Authentik, Keycloak, etc.) diff --git a/config/test.exs b/config/test.exs index 7343a6a..ef54982 100644 --- a/config/test.exs +++ b/config/test.exs @@ -62,7 +62,3 @@ config :mv, :join_rate_limit, scale_ms: 60_000, limit: 2 # Ash: silence "after_transaction hooks in surrounding transaction" warning when using # Ecto sandbox (tests run in a transaction; create_member after_transaction is expected). config :ash, warn_on_transaction_hooks?: false - -# StreamData property tests: generated cases per property, overridable via PROPERTY_RUNS -# (the `just check` recipe sets it low for speed; default 100 otherwise). -config :stream_data, max_runs: String.to_integer(System.get_env("PROPERTY_RUNS") || "100") diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 98d4053..37f9552 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -33,7 +33,7 @@ services: restart: unless-stopped db-prod: - image: postgres:18.4-alpine + image: postgres:18.3-alpine container_name: mv-prod-db environment: POSTGRES_USER: postgres diff --git a/docker-compose.yml b/docker-compose.yml index cbd2e9e..512626b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,7 +4,7 @@ networks: services: db: - image: postgres:18.4-alpine + image: postgres:18.3-alpine environment: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres @@ -25,7 +25,7 @@ services: rauthy: container_name: rauthy-dev - image: ghcr.io/sebadob/rauthy:0.35.2 + image: ghcr.io/sebadob/rauthy:0.35.1 environment: - LOCAL_TEST=true - SMTP_URL=mailcrab @@ -36,9 +36,6 @@ services: - BOOTSTRAP_ADMIN_PASSWORD_PLAIN=RauthyTest12345 # Disable strict IP validation to allow access from multiple Docker networks - SESSION_VALIDATE_IP=false - # Auto-seed the `mv` OIDC client (id + plain secret) on first DB init. - # Re-runs after `docker compose down -v` because the DB is empty again. - - BOOTSTRAP_DIR=/app/bootstrap ports: - "8080:8080" depends_on: @@ -49,7 +46,6 @@ services: - local volumes: - rauthy-data:/app/data - - ./rauthy-bootstrap:/app/bootstrap:ro volumes: postgres-data: diff --git a/docs/database-schema-readme.md b/docs/database-schema-readme.md index fa6ea55..f58cbea 100644 --- a/docs/database-schema-readme.md +++ b/docs/database-schema-readme.md @@ -188,6 +188,7 @@ Settings (1) → MembershipFeeType (0..1) ### Member Constraints - First name and last name required (min 1 char) - Email unique, validated format (5-254 chars) +- Join date cannot be in future - Exit date must be after join date - Phone: `+?[0-9\- ]{6,20}` - Postal code: optional (no format validation) diff --git a/docs/database_schema.dbml b/docs/database_schema.dbml index 16c9723..61da063 100644 --- a/docs/database_schema.dbml +++ b/docs/database_schema.dbml @@ -124,7 +124,7 @@ Table members { first_name text [null, note: 'Member first name (min length: 1 if present)'] last_name text [null, note: 'Member last name (min length: 1 if present)'] email text [not null, unique, note: 'Member email address (5-254 chars, validated)'] - join_date date [null, note: 'Date when member joined club'] + join_date date [null, note: 'Date when member joined club (cannot be in future)'] exit_date date [null, note: 'Date when member left club (must be after join_date)'] notes text [null, note: 'Additional notes about member'] city text [null, note: 'City of residence'] @@ -187,6 +187,7 @@ Table members { **Validation Rules:** - first_name, last_name: optional, but if present min 1 character - email: 5-254 characters, valid email format (required) + - join_date: cannot be in future - exit_date: must be after join_date (if both present) - postal_code: optional (no format validation) - country: optional diff --git a/lib/membership/custom_field.ex b/lib/membership/custom_field.ex index 5f4dd0e..ef6c79a 100644 --- a/lib/membership/custom_field.ex +++ b/lib/membership/custom_field.ex @@ -12,8 +12,6 @@ defmodule Mv.Membership.CustomField do - `slug` - URL-friendly, immutable identifier automatically generated from name (e.g., "phone-mobile") - `value_type` - Data type constraint (`:string`, `:integer`, `:boolean`, `:date`, `:email`). Immutable after creation. - `description` - Optional human-readable description - - `join_description` - Optional label shown for this field on the public join form - (e.g., a GDPR confirmation text); supports inline external links. Falls back to `name` when nil. - `required` - If true, all members must have this custom field (future feature) - `show_in_overview` - If true, this custom field will be displayed in the member overview table and can be sorted @@ -63,14 +61,7 @@ defmodule Mv.Membership.CustomField do end actions do - default_accept [ - :name, - :value_type, - :description, - :join_description, - :required, - :show_in_overview - ] + default_accept [:name, :value_type, :description, :required, :show_in_overview] read :read do primary? true @@ -78,13 +69,13 @@ defmodule Mv.Membership.CustomField do end create :create do - accept [:name, :value_type, :description, :join_description, :required, :show_in_overview] + accept [:name, :value_type, :description, :required, :show_in_overview] change Mv.Membership.Changes.GenerateSlug validate string_length(:slug, min: 1) end update :update do - accept [:name, :description, :join_description, :required, :show_in_overview] + accept [:name, :description, :required, :show_in_overview] require_atomic? false validate fn changeset, _context -> @@ -148,15 +139,6 @@ defmodule Mv.Membership.CustomField do trim?: true ] - attribute :join_description, :string, - allow_nil?: true, - public?: true, - description: "Label shown for this field on the public join form; supports external links", - constraints: [ - max_length: 1000, - trim?: true - ] - attribute :required, :boolean, default: false, allow_nil?: false 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 8dae2d1..5de15c8 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,10 +17,16 @@ defmodule Mv.Membership.JoinRequest.Changes.FilterFormDataByAllowlist do form_data = Ash.Changeset.get_attribute(changeset, :form_data) || %{} allowlist_ids = - Membership.get_join_form_allowlist() - |> Enum.map(fn item -> item.id end) - |> MapSet.new() - |> MapSet.difference(MapSet.new(@typed_fields)) + 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 filtered = form_data diff --git a/lib/membership/member.ex b/lib/membership/member.ex index cddc23f..4e85fa8 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -22,7 +22,7 @@ defmodule Mv.Membership.Member do ## Validations - Required: email (all other fields are optional) - Email format validation (using EctoCommons.EmailValidator) - - Date validations: exit_date after join_date + - Date validations: join_date not in future, exit_date after join_date - Email uniqueness: prevents conflicts with unlinked users - Linked member email change: only admins or the linked user may change a linked member's email (see `Mv.Membership.Member.Validations.EmailChangePermission`) @@ -51,9 +51,6 @@ defmodule Mv.Membership.Member do require Logger - @typedoc "An `Mv.Membership.Member` resource record." - @type t :: %__MODULE__{} - # Module constants @member_search_limit 10 @@ -476,6 +473,11 @@ defmodule Mv.Membership.Member do end end + # Join date not in future + validate compare(:join_date, less_than_or_equal_to: &Date.utc_today/0), + where: [present(:join_date)], + message: "cannot be in the future" + # Exit date not before join date validate compare(:exit_date, greater_than: :join_date), where: [present([:join_date, :exit_date])], @@ -794,7 +796,7 @@ defmodule Mv.Membership.Member do # nil/[] when membership_fee_type is missing. @doc false - @spec get_current_cycle(t()) :: MembershipFeeCycle.t() | nil + @spec get_current_cycle(Member.t()) :: MembershipFeeCycle.t() | nil def get_current_cycle(member) do today = Date.utc_today() @@ -824,7 +826,7 @@ defmodule Mv.Membership.Member do end @doc false - @spec get_last_completed_cycle(t()) :: MembershipFeeCycle.t() | nil + @spec get_last_completed_cycle(Member.t()) :: MembershipFeeCycle.t() | nil def get_last_completed_cycle(member) do today = Date.utc_today() @@ -870,7 +872,7 @@ defmodule Mv.Membership.Member do end @doc false - @spec get_overdue_cycles(t()) :: [MembershipFeeCycle.t()] + @spec get_overdue_cycles(Member.t()) :: [MembershipFeeCycle.t()] def get_overdue_cycles(member) do today = Date.utc_today() @@ -942,7 +944,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 @@ -950,7 +952,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} -> @@ -1096,7 +1098,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, @@ -1115,7 +1117,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, @@ -1234,6 +1236,8 @@ defmodule Mv.Membership.Member do |> String.replace("_", "\\_") end + defp sanitize_search_query(_), do: "" + # ============================================================================ # Search Filter Builders # ============================================================================ 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 da8a291..dc4d097 100644 --- a/lib/membership/member/changes/unrelate_user_when_argument_nil.ex +++ b/lib/membership/member/changes/unrelate_user_when_argument_nil.ex @@ -37,10 +37,9 @@ 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/membership/membership.ex b/lib/membership/membership.ex index 72be69b..7fa35dc 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -836,10 +836,7 @@ 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()} - | {:ok, JoinRequest.t(), [Ash.Notifier.Notification.t()]} - | {:error, term()} + @spec reject_join_request(String.t(), keyword()) :: {:ok, JoinRequest.t()} | {:error, term()} def reject_join_request(id, opts \\ []) do actor = Keyword.get(opts, :actor) 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 8f5aa56..0e9cf00 100644 --- a/lib/membership_fees/changes/set_membership_fee_start_date.ex +++ b/lib/membership_fees/changes/set_membership_fee_start_date.ex @@ -26,6 +26,8 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do """ use Ash.Resource.Change + require Logger + alias Mv.MembershipFees.CalendarCycles @impl true @@ -81,6 +83,11 @@ 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/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index ae84cdb..3ffae93 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -43,7 +43,6 @@ 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 @@ -89,7 +88,7 @@ defmodule Mv.Authorization.PermissionSets do iex> PermissionSets.all_permission_sets() [:own_data, :read_only, :normal_user, :admin] """ - @spec all_permission_sets() :: [permission_set_name(), ...] + @spec all_permission_sets() :: [atom()] def all_permission_sets do [:own_data, :read_only, :normal_user, :admin] end @@ -108,7 +107,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(permission_set_name()) :: permission_set() + @spec get_permissions(atom()) :: 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 750a7db..870d1d3 100644 --- a/lib/mv/config.ex +++ b/lib/mv/config.ex @@ -207,6 +207,8 @@ 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). """ @@ -249,6 +251,7 @@ defmodule Mv.Config do case System.get_env(key) do nil -> false v when is_binary(v) -> String.trim(v) != "" + _ -> false end end @@ -267,6 +270,9 @@ defmodule Mv.Config do value when is_binary(value) -> v = String.trim(value) |> String.downcase() v in ["true", "1", "yes"] + + _ -> + false end end @@ -322,6 +328,7 @@ 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 @@ -402,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() + @spec oidc_groups_claim() :: String.t() | nil def oidc_groups_claim do case env_or_setting("OIDC_GROUPS_CLAIM", :oidc_groups_claim) do nil -> "groups" @@ -485,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() :: pos_integer() | nil + @spec smtp_port() :: non_neg_integer() | nil def smtp_port do if smtp_env_mode?() do parse_smtp_port_env(System.get_env("SMTP_PORT")) @@ -631,15 +638,9 @@ defmodule Mv.Config do """ @spec mail_from_name() :: String.t() def mail_from_name do - 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 + case System.get_env("MAIL_FROM_NAME") do + nil -> get_from_settings(:smtp_from_name) || "Mila" + value -> trim_nil(value) || "Mila" end end diff --git a/lib/mv/constants.ex b/lib/mv/constants.ex index 657aa9b..517ad2f 100644 --- a/lib/mv/constants.ex +++ b/lib/mv/constants.ex @@ -26,22 +26,8 @@ defmodule Mv.Constants do @fee_type_filter_prefix "fee_type_" - @join_date_from_param "jd_from" - - @join_date_to_param "jd_to" - - @exit_date_mode_param "ed_mode" - - @exit_date_from_param "ed_from" - - @exit_date_to_param "ed_to" - - @custom_date_filter_prefix "cdf_" - @max_boolean_filters 50 - @max_mailto_bulk_recipients 50 - @max_uuid_length 36 @email_validator_checks [:html_input, :pow] @@ -98,70 +84,6 @@ defmodule Mv.Constants do """ def fee_type_filter_prefix, do: @fee_type_filter_prefix - @doc """ - Returns the URL parameter name for the join_date lower bound filter. - - ## Examples - - iex> Mv.Constants.join_date_from_param() - "jd_from" - """ - def join_date_from_param, do: @join_date_from_param - - @doc """ - Returns the URL parameter name for the join_date upper bound filter. - - ## Examples - - iex> Mv.Constants.join_date_to_param() - "jd_to" - """ - def join_date_to_param, do: @join_date_to_param - - @doc """ - Returns the URL parameter name for the exit_date filter mode - (`active_only` | `inactive_only` | `all` | `custom`). - - ## Examples - - iex> Mv.Constants.exit_date_mode_param() - "ed_mode" - """ - def exit_date_mode_param, do: @exit_date_mode_param - - @doc """ - Returns the URL parameter name for the exit_date lower bound filter - (only relevant when ed_mode=custom). - - ## Examples - - iex> Mv.Constants.exit_date_from_param() - "ed_from" - """ - def exit_date_from_param, do: @exit_date_from_param - - @doc """ - Returns the URL parameter name for the exit_date upper bound filter - (only relevant when ed_mode=custom). - - ## Examples - - iex> Mv.Constants.exit_date_to_param() - "ed_to" - """ - def exit_date_to_param, do: @exit_date_to_param - - @doc """ - Returns the prefix for custom date field filter URL parameters - (e.g. cdf__from / cdf__to). - - ## Examples - - iex> Mv.Constants.custom_date_filter_prefix() - "cdf_" - """ - def custom_date_filter_prefix, do: @custom_date_filter_prefix - @doc """ Returns the maximum number of boolean custom field filters allowed per request. @@ -175,21 +97,6 @@ defmodule Mv.Constants do """ def max_boolean_filters, do: @max_boolean_filters - @doc """ - Returns the maximum number of mailto recipients before the bulk "open in email - program" action is disabled. - - The mailto link carries every recipient in its BCC; browsers cannot reliably - hand a too-long mailto URI to the mail program. At or above this count the - action is disabled in the UI (Copy and Export have no such limit). - - ## Examples - - iex> Mv.Constants.max_mailto_bulk_recipients() - 50 - """ - def max_mailto_bulk_recipients, do: @max_mailto_bulk_recipients - @doc """ Returns the maximum length of a UUID string (36 characters including hyphens). diff --git a/lib/mv/helpers/system_actor.ex b/lib/mv/helpers/system_actor.ex index 7b86a3c..8cd93d2 100644 --- a/lib/mv/helpers/system_actor.ex +++ b/lib/mv/helpers/system_actor.ex @@ -225,10 +225,7 @@ 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 - case System.get_env("SYSTEM_ACTOR_EMAIL") do - nil -> "system@mila.local" - email -> email - end + System.get_env("SYSTEM_ACTOR_EMAIL") || "system@mila.local" end # Loads the system actor from the database @@ -260,7 +257,7 @@ defmodule Mv.Helpers.SystemActor do end # Handles database error when loading system user - @spec handle_system_user_error({:error, Ash.Error.t()}) :: Mv.Accounts.User.t() | no_return() + @spec handle_system_user_error(term()) :: Mv.Accounts.User.t() | no_return() defp handle_system_user_error(error) do case load_admin_user_fallback() do {:ok, admin_user} -> @@ -396,18 +393,15 @@ 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 - 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 + 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) end # Finds a user by email address diff --git a/lib/mv/mailer.ex b/lib/mv/mailer.ex index 1e55b6e..ec8f357 100644 --- a/lib/mv/mailer.ex +++ b/lib/mv/mailer.ex @@ -190,4 +190,6 @@ 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/custom_field_value_formatter.ex b/lib/mv/membership/custom_field_value_formatter.ex index 9ba9c42..9709353 100644 --- a/lib/mv/membership/custom_field_value_formatter.ex +++ b/lib/mv/membership/custom_field_value_formatter.ex @@ -4,13 +4,13 @@ defmodule Mv.Membership.CustomFieldValueFormatter do Same logic as the member overview Formatter but without Gettext or web helpers, so it can be used from the Membership context. For boolean: "Yes"/"No"; - for date: ISO-8601 (YYYY-MM-DD) so exported values can be re-imported. + for date: European format (dd.mm.yyyy). """ @doc """ Formats a custom field value for plain text (e.g. CSV). Handles nil, Ash.Union, JSONB map, and direct values. Uses custom_field.value_type - for typing. Boolean -> "Yes"/"No", Date -> ISO-8601 (YYYY-MM-DD). + for typing. Boolean -> "Yes"/"No", Date -> dd.mm.yyyy. """ def format_custom_field_value(nil, _custom_field), do: "" @@ -18,10 +18,6 @@ defmodule Mv.Membership.CustomFieldValueFormatter do format_value_by_type(value, type, custom_field) end - def format_custom_field_value(%Date{} = value, custom_field) do - format_value_by_type(value, :date, custom_field) - end - def format_custom_field_value(value, custom_field) when is_map(value) do type = Map.get(value, "type") || Map.get(value, "_union_type") val = Map.get(value, "value") || Map.get(value, "_union_value") @@ -45,12 +41,12 @@ defmodule Mv.Membership.CustomFieldValueFormatter do defp format_value_by_type(value, :boolean, _), do: to_string(value) defp format_value_by_type(%Date{} = date, :date, _) do - Date.to_iso8601(date) + Calendar.strftime(date, "%d.%m.%Y") end defp format_value_by_type(value, :date, _) when is_binary(value) do case Date.from_iso8601(value) do - {:ok, date} -> Date.to_iso8601(date) + {:ok, date} -> Calendar.strftime(date, "%d.%m.%Y") _ -> value end end diff --git a/lib/mv/membership/import/column_resolver.ex b/lib/mv/membership/import/column_resolver.ex deleted file mode 100644 index 2edb540..0000000 --- a/lib/mv/membership/import/column_resolver.ex +++ /dev/null @@ -1,258 +0,0 @@ -defmodule Mv.Membership.Import.ColumnResolver do - @moduledoc """ - Read-only resolution of CSV import columns against the database. - - Given the `HeaderMapper.build_maps/2` result, the raw numbered rows, and an - actor, `resolve/3` determines: - - - which group names in the groups column already exist (`groups_found`) and - which would have to be created (`groups_to_create`); - - a small set of preview rows for the mapping preview UI. - - No database writes happen here; the resolver only reads. Group creation and - member-group assignment happen during processing via `create_or_find_group/3`. - - This module has no Phoenix or web dependencies. - """ - - require Logger - - alias Mv.Membership.Import.HeaderMapper - - @preview_row_limit 3 - - @type numbered_row :: {pos_integer(), [String.t()]} - - @type resolution :: %{ - groups_found: [%{id: String.t(), name: String.t()}], - groups_to_create: [String.t()], - fee_type_map: %{String.t() => String.t()}, - fee_type_warnings: [String.t()], - has_empty_fee_type_cells?: boolean(), - preview_rows: [[String.t()]] - } - - @doc """ - Resolves the group and fee-type columns of an import against the database and - extracts preview rows. - - Returns a map with `:groups_found`, `:groups_to_create`, `:fee_type_map`, - `:fee_type_warnings`, `:has_empty_fee_type_cells?`, and `:preview_rows`. - """ - @spec resolve(map(), [numbered_row()], term()) :: resolution() - def resolve(header_maps, rows, actor) do - %{ - groups_found: groups_found, - groups_to_create: groups_to_create - } = resolve_groups(header_maps, rows, actor) - - %{ - fee_type_map: fee_type_map, - fee_type_warnings: fee_type_warnings, - has_empty_fee_type_cells?: has_empty_fee_type_cells? - } = resolve_fee_types(header_maps, rows, actor) - - %{ - groups_found: groups_found, - groups_to_create: groups_to_create, - fee_type_map: fee_type_map, - fee_type_warnings: fee_type_warnings, - has_empty_fee_type_cells?: has_empty_fee_type_cells?, - preview_rows: preview_rows(rows) - } - end - - defp resolve_groups(%{groups_column_index: nil}, _rows, _actor) do - %{groups_found: [], groups_to_create: []} - end - - defp resolve_groups(%{groups_column_index: index}, rows, actor) do - existing_groups = list_groups(actor) - lookup = build_group_lookup(existing_groups) - - names = unique_group_names(rows, index) - - {found, to_create} = - Enum.reduce(names, {[], []}, fn name, {found, to_create} -> - case Map.get(lookup, normalize_name(name)) do - nil -> {found, [name | to_create]} - group -> {[%{id: group.id, name: group.name} | found], to_create} - end - end) - - %{groups_found: Enum.reverse(found), groups_to_create: Enum.reverse(to_create)} - end - - defp resolve_fee_types(%{fee_type_column_index: nil}, _rows, _actor) do - %{fee_type_map: %{}, fee_type_warnings: [], has_empty_fee_type_cells?: false} - end - - defp resolve_fee_types(%{fee_type_column_index: index}, rows, actor) do - lookup = build_fee_type_lookup(actor) - - cells = Enum.map(rows, fn {_line, values} -> Enum.at(values, index) end) - - has_empty? = Enum.any?(cells, &blank?/1) - - {fee_type_map, warnings} = - cells - |> Enum.reject(&blank?/1) - |> Enum.uniq_by(&normalize_fee_type_name/1) - |> Enum.reduce({%{}, []}, fn name, {map, warnings} -> - case Map.get(lookup, normalize_fee_type_name(name)) do - nil -> {map, [String.trim(name) | warnings]} - id -> {Map.put(map, normalize_fee_type_name(name), id), warnings} - end - end) - - %{ - fee_type_map: fee_type_map, - fee_type_warnings: Enum.reverse(warnings), - has_empty_fee_type_cells?: has_empty? - } - end - - @doc """ - Normalizes a fee-type name using the same rules as CSV header normalization - (trim, lowercase, transliterate, drop hyphens and whitespace). - """ - @spec normalize_fee_type_name(String.t() | nil) :: String.t() - def normalize_fee_type_name(name) when is_binary(name), do: HeaderMapper.normalize_header(name) - def normalize_fee_type_name(_), do: "" - - defp build_fee_type_lookup(actor) do - actor - |> list_fee_types() - |> Enum.reduce(%{}, fn fee_type, acc -> - normalized = normalize_fee_type_name(fee_type.name) - - if Map.has_key?(acc, normalized) do - Logger.warning( - "Multiple membership fee types normalize to #{inspect(normalized)}; using the first match for CSV import." - ) - - acc - else - Map.put(acc, normalized, fee_type.id) - end - end) - end - - defp list_fee_types(actor) do - Mv.MembershipFees.list_membership_fee_types!(actor: actor) - end - - defp blank?(nil), do: true - defp blank?(value) when is_binary(value), do: String.trim(value) == "" - defp blank?(_), do: false - - @doc """ - Finds an existing group by name (case-insensitive) or creates it. - - Looks first in the pre-fetched `groups` list, then in the database (to catch - groups created earlier in the same import), and only creates a new group when - none is found. This keeps group resolution idempotent across re-imports. - """ - @spec create_or_find_group(String.t(), [Mv.Membership.Group.t()], term()) :: - {:ok, Mv.Membership.Group.t()} | {:error, term()} - def create_or_find_group(name, groups, actor) when is_binary(name) do - trimmed = String.trim(name) - normalized = normalize_name(trimmed) - - case find_group_in_list(groups, normalized) do - nil -> find_or_create_group(trimmed, normalized, actor) - group -> {:ok, group} - end - end - - defp find_group_in_list(groups, normalized) do - Enum.find(groups, fn group -> normalize_name(group.name) == normalized end) - end - - defp find_or_create_group(trimmed, normalized, actor) do - case fetch_group_by_normalized_name(normalized, actor) do - nil -> create_group(trimmed, normalized, actor) - group -> {:ok, group} - end - end - - # Normalizes the Ash code-interface return to a two-shape result. - # - # On a create failure the group may have been created concurrently by another - # import session between our read and our write (the DB unique index is the - # final arbiter, and the name validation is fail-open). Re-fetch by normalized - # name and link to the existing group rather than failing the row. - defp create_group(name, normalized, actor) do - case Mv.Membership.create_group(%{name: name}, actor: actor) do - {:ok, %Mv.Membership.Group{} = group} -> - {:ok, group} - - {:error, reason} -> - case fetch_group_by_normalized_name(normalized, actor) do - nil -> {:error, reason} - group -> {:ok, group} - end - end - end - - # Fetches a single group by case-insensitive name using a name-filtered query - # rather than reading the whole groups table. `normalized` is the trimmed, - # lower-cased name; the DB comparison uses LOWER(name) consistent with the - # Group resource's case-insensitive uniqueness constraint. - defp fetch_group_by_normalized_name(normalized, actor) do - require Ash.Query - - Mv.Membership.Group - |> Ash.Query.filter(fragment("LOWER(?) = ?", name, ^normalized)) - |> Ash.read(actor: actor, domain: Mv.Membership) - |> case do - {:ok, [group | _]} -> group - _ -> nil - end - end - - @doc """ - Splits a raw groups-cell value into trimmed, non-empty group names. - """ - @spec split_group_names(String.t() | nil) :: [String.t()] - def split_group_names(nil), do: [] - - def split_group_names(cell) when is_binary(cell) do - cell - |> String.split(",") - |> Enum.map(&String.trim/1) - |> Enum.reject(&(&1 == "")) - end - - defp unique_group_names(rows, index) do - rows - |> Enum.flat_map(fn {_line, values} -> - values - |> Enum.at(index) - |> split_group_names() - end) - |> Enum.uniq_by(&normalize_name/1) - end - - defp preview_rows(rows) do - rows - |> Enum.take(@preview_row_limit) - |> Enum.map(fn {_line, values} -> values end) - end - - defp list_groups(actor) do - Mv.Membership.list_groups!(actor: actor) - end - - defp build_group_lookup(groups) do - Enum.reduce(groups, %{}, fn group, acc -> - Map.put(acc, normalize_name(group.name), group) - end) - end - - # Case-insensitive comparison consistent with the Group resource's - # case-insensitive name uniqueness. - defp normalize_name(name) when is_binary(name) do - name |> String.trim() |> String.downcase() - end -end diff --git a/lib/mv/membership/import/csv_parser.ex b/lib/mv/membership/import/csv_parser.ex index 142450f..2de75ee 100644 --- a/lib/mv/membership/import/csv_parser.ex +++ b/lib/mv/membership/import/csv_parser.ex @@ -100,8 +100,7 @@ defmodule Mv.Membership.Import.CsvParser do |> String.replace("\r", "\n") end - @spec get_parser(String.t()) :: - Mv.Membership.Import.CsvParserSemicolon | Mv.Membership.Import.CsvParserComma + @spec get_parser(String.t()) :: module() defp get_parser(";"), do: Mv.Membership.Import.CsvParserSemicolon defp get_parser(","), do: Mv.Membership.Import.CsvParserComma defp get_parser(_), do: Mv.Membership.Import.CsvParserSemicolon @@ -117,10 +116,7 @@ defmodule Mv.Membership.Import.CsvParser do if semicolon_score >= comma_score, do: ";", else: "," end - @spec header_field_count( - Mv.Membership.Import.CsvParserSemicolon | Mv.Membership.Import.CsvParserComma, - binary() - ) :: non_neg_integer() + @spec header_field_count(module(), 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/import/header_mapper.ex b/lib/mv/membership/import/header_mapper.ex index be90ca6..d96d96e 100644 --- a/lib/mv/membership/import/header_mapper.ex +++ b/lib/mv/membership/import/header_mapper.ex @@ -29,21 +29,12 @@ defmodule Mv.Membership.Import.HeaderMapper do Supports English and German header variants (e.g. "Email" / "E-Mail", "Join Date" / "Beitrittsdatum"). - ## Special columns - - - **groups** – Many-to-many relationship (through member_groups). Recognized via the - `groups_column_index` key (headers `Groups`, `Gruppen`, `Gruppe`). Comma-separated - names are resolved during processing; missing groups are auto-created. - - **membership_fee_type** – Recognized via the `fee_type_column_index` key (headers - `Fee Type`, `fee_type`, `membership_fee_type`, `Beitragsart`). Names are matched to - existing fee types; unknown names fall back to the default fee type. - ## Fields not supported for import - **membership_fee_status** – Computed (calculation from membership fee cycles). Not stored; - cannot be set via CSV. Export can include it. Fee-status header variants - (`Membership Fee Status`, `Bezahlstatus`, `Mitgliedsbeitragsstatus`) are explicitly - placed in the `ignored` list and never mapped. + cannot be set via CSV. Export can include it. + - **groups** – Many-to-many relationship (through member_groups). Import would require + resolving group names/slugs to IDs and creating associations; not in current import scope. ## Custom Field Detection @@ -56,10 +47,10 @@ defmodule Mv.Membership.Import.HeaderMapper do "e-mail" iex> HeaderMapper.build_maps(["Email", "First Name"], []) - {:ok, %{member: %{email: 0, first_name: 1}, custom: %{}, unknown: [], ignored: [], groups_column_index: nil, fee_type_column_index: nil}} + {:ok, %{member: %{email: 0, first_name: 1}, custom: %{}, unknown: []}} iex> HeaderMapper.build_maps(["Email", "CustomField"], [%{id: "cf1", name: "CustomField"}]) - {:ok, %{member: %{email: 0}, custom: %{"cf1" => 1}, unknown: [], ignored: [], groups_column_index: nil, fee_type_column_index: nil}} + {:ok, %{member: %{email: 0}, custom: %{"cf1" => 1}, unknown: []}} """ @type column_map :: %{atom() => non_neg_integer()} @@ -69,33 +60,6 @@ defmodule Mv.Membership.Import.HeaderMapper do # Required member fields @required_member_fields [:email] - # Fee-status header variants that must never be imported (computed/read-only field). - # Stored already-normalized; checked before member, custom, groups, and fee-type mapping. - # Maintain this list when new locale translations for fee-status are added. - @ignored_normalized [ - "membershipfeestatus", - "mitgliedsbeitragsstatus", - "bezahlstatus", - # DE export label for membership_fee_start_date — system-managed, not importable - "startdatummitgliedsbeitrag" - ] - - # Normalized header variants for the groups column. The column is resolved to - # group associations during import; it is never a member or custom field. - @groups_column_normalized [ - "groups", - "gruppen", - "gruppe" - ] - - # Normalized header variants for the membership fee-type column. The column is - # resolved to a MembershipFeeType during import; it is never a member or custom field. - @fee_type_column_normalized [ - "membershipfeetype", - "feetype", - "beitragsart" - ] - # Canonical member fields with their raw variants # These will be normalized at runtime when building the lookup map @member_field_variants_raw %{ @@ -275,79 +239,30 @@ defmodule Mv.Membership.Import.HeaderMapper do ## Returns - - `{:ok, %{member: column_map, custom: custom_field_map, unknown: unknown_headers, - ignored: [non_neg_integer], groups_column_index: non_neg_integer | nil, - fee_type_column_index: non_neg_integer | nil}}` on success + - `{:ok, %{member: column_map, custom: custom_field_map, unknown: unknown_headers}}` on success - `{:error, reason}` on error (missing required field, duplicate headers) - The `ignored` list holds the indices of fee-status columns (computed/read-only), - which are never mapped to member or custom fields. - ## Examples iex> build_maps(["Email", "First Name"], []) - {:ok, %{member: %{email: 0, first_name: 1}, custom: %{}, unknown: [], ignored: [], groups_column_index: nil, fee_type_column_index: nil}} + {:ok, %{member: %{email: 0, first_name: 1}, custom: %{}, unknown: []}} iex> build_maps(["Email", "CustomField"], [%{id: "cf1", name: "CustomField"}]) - {:ok, %{member: %{email: 0}, custom: %{"cf1" => 1}, unknown: [], ignored: [], groups_column_index: nil, fee_type_column_index: nil}} + {:ok, %{member: %{email: 0}, custom: %{"cf1" => 1}, unknown: []}} """ @spec build_maps([String.t()], [map()]) :: - {:ok, - %{ - member: column_map(), - custom: custom_field_map(), - unknown: unknown_headers(), - ignored: [non_neg_integer()], - groups_column_index: non_neg_integer() | nil, - fee_type_column_index: non_neg_integer() | nil - }} + {:ok, %{member: column_map(), custom: custom_field_map(), unknown: unknown_headers()}} | {:error, String.t()} def build_maps(headers, custom_fields) when is_list(headers) and is_list(custom_fields) do - ignored = ignored_indices(headers) - groups_column_index = first_matching_index(headers, @groups_column_normalized) - fee_type_column_index = first_matching_index(headers, @fee_type_column_normalized) - - reserved = - [groups_column_index, fee_type_column_index | ignored] - |> Enum.reject(&is_nil/1) - |> MapSet.new() - - with {:ok, member_map, unknown_after_member} <- build_member_map(headers, reserved), + with {:ok, member_map, unknown_after_member} <- build_member_map(headers), {:ok, custom_map, unknown_after_custom} <- build_custom_field_map(headers, unknown_after_member, custom_fields, member_map) do unknown = Enum.map(unknown_after_custom, &Enum.at(headers, &1)) - - {:ok, - %{ - member: member_map, - custom: custom_map, - unknown: unknown, - ignored: ignored, - groups_column_index: groups_column_index, - fee_type_column_index: fee_type_column_index - }} + {:ok, %{member: member_map, custom: custom_map, unknown: unknown}} end end - # Returns the index of the first header whose normalized form is in `variants`, - # or nil if none match. - defp first_matching_index(headers, variants) do - headers - |> Enum.with_index() - |> Enum.find_value(fn {header, index} -> - if normalize_header(header) in variants, do: index - end) - end - - # Returns the column indices whose normalized header is in the fee-status ignore list. - defp ignored_indices(headers) do - headers - |> Enum.with_index() - |> Enum.filter(fn {header, _index} -> normalize_header(header) in @ignored_normalized end) - |> Enum.map(fn {_header, index} -> index end) - end - # --- Private Functions --- # Transliterates German umlauts and special characters @@ -389,14 +304,13 @@ defmodule Mv.Membership.Import.HeaderMapper do |> String.replace(" ", "") end - # Builds member field column map, skipping reserved (e.g. ignored) indices. - defp build_member_map(headers, reserved) do + # Builds member field column map + defp build_member_map(headers) do result = headers |> Enum.with_index() |> Enum.reduce_while({%{}, []}, fn {header, index}, {acc_map, acc_unknown} -> - normalized = - if MapSet.member?(reserved, index), do: "", else: normalize_header(header) + normalized = normalize_header(header) case process_member_header(header, index, normalized, acc_map, %{}) do {:error, reason} -> diff --git a/lib/mv/membership/import/import_runner.ex b/lib/mv/membership/import/import_runner.ex index 28893a3..eccd75f 100644 --- a/lib/mv/membership/import/import_runner.ex +++ b/lib/mv/membership/import/import_runner.ex @@ -26,8 +26,14 @@ 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, to_string(:file.format_error(reason))} + {:error, Exception.message(reason)} end end @@ -80,7 +86,7 @@ defmodule Mv.Membership.Import.ImportRunner do all_errors = progress.errors ++ chunk_result.errors new_errors = Enum.take(all_errors, max_errors) errors_truncated? = length(all_errors) > max_errors - new_warnings = Enum.uniq(progress.warnings ++ Map.get(chunk_result, :warnings, [])) + new_warnings = progress.warnings ++ Map.get(chunk_result, :warnings, []) chunks_processed = current_chunk_idx + 1 new_status = if chunks_processed >= progress.total_chunks, do: :done, else: :running @@ -97,20 +103,6 @@ defmodule Mv.Membership.Import.ImportRunner do } end - @doc """ - Carries the in-memory group snapshot grown by a chunk back into `import_state` - so the next chunk reuses groups created earlier instead of re-reading the - Group table. When the chunk result omits `groups_found`, the state is returned - unchanged. - """ - @spec carry_groups_forward(map(), map()) :: map() - def carry_groups_forward(import_state, chunk_result) do - case Map.fetch(chunk_result, :groups_found) do - {:ok, groups_found} -> Map.put(import_state, :groups_found, groups_found) - :error -> import_state - end - end - @doc """ Returns the next action after processing a chunk: send the next chunk index or done. """ diff --git a/lib/mv/membership/import/member_csv.ex b/lib/mv/membership/import/member_csv.ex index 31dea59..23e0d93 100644 --- a/lib/mv/membership/import/member_csv.ex +++ b/lib/mv/membership/import/member_csv.ex @@ -6,7 +6,7 @@ defmodule Mv.Membership.Import.MemberCSV do This module provides the core API for CSV member import functionality: - `prepare/2` - Parses and validates CSV content, returns import state - - `process_chunk/4` - Processes a chunk of rows and creates members + - `process_chunk/3` - Processes a chunk of rows and creates members ## Error Handling @@ -22,24 +22,13 @@ defmodule Mv.Membership.Import.MemberCSV do - `column_map` - Map of canonical field names to column indices - `custom_field_map` - Map of custom field names to column indices - `warnings` - List of warning messages (e.g., unknown custom field columns) - - `headers` - The raw CSV header row - - `ignored` - Header names of ignored (fee-status) columns - - `groups_column_index` / `fee_type_column_index` - Indices for resolved columns (or nil) - - `groups_found` / `groups_to_create` - Existing and to-be-created groups from the preview - - `fee_type_map` - Normalized fee-type name to id, for matched fee types - - `fee_type_warnings` - Unmatched fee-type names surfaced in the preview - - `has_empty_fee_type_cells?` - Whether any fee-type cell is blank (default applies) - - `preview_rows` - Up to 3 sample data rows for the mapping preview ## Chunk Results - The `chunk_result` returned by `process_chunk/4` contains: + The `chunk_result` returned by `process_chunk/3` contains: - `inserted` - Number of successfully created members - `failed` - Number of failed member creations - `errors` - List of `%MemberCSV.Error{}` structs (capped at 50 per import) - - `groups_found` - The in-memory group snapshot grown while processing this - chunk; thread it into the next chunk's `:groups_found` opt so groups created - in an earlier chunk are reused without re-reading the Group table ## Examples @@ -48,9 +37,7 @@ defmodule Mv.Membership.Import.MemberCSV do # Process first chunk chunk = Enum.at(import_state.chunks, 0) - - {:ok, result} = - MemberCSV.process_chunk(chunk, import_state.column_map, import_state.custom_field_map, []) + {:ok, result} = MemberCSV.process_chunk(chunk, import_state.column_map) """ defmodule Error do @@ -79,29 +66,16 @@ defmodule Mv.Membership.Import.MemberCSV do custom_field_lookup: %{ String.t() => %{id: String.t(), value_type: atom(), name: String.t()} }, - warnings: list(String.t()), - headers: list(String.t()), - ignored: list(String.t()), - groups_column_index: non_neg_integer() | nil, - fee_type_column_index: non_neg_integer() | nil, - groups_found: list(%{id: String.t(), name: String.t()}), - groups_to_create: list(String.t()), - fee_type_map: %{String.t() => String.t()}, - fee_type_warnings: list(String.t()), - has_empty_fee_type_cells?: boolean(), - preview_rows: list(list(String.t())) + warnings: list(String.t()) } @type chunk_result :: %{ inserted: non_neg_integer(), failed: non_neg_integer(), errors: list(Error.t()), - errors_truncated?: boolean(), - warnings: list(String.t()), - groups_found: list(Mv.Membership.Group.t() | %{id: String.t(), name: String.t()}) + errors_truncated?: boolean() } - alias Mv.Membership.Import.ColumnResolver alias Mv.Membership.Import.CsvParser alias Mv.Membership.Import.HeaderMapper @@ -165,27 +139,13 @@ defmodule Mv.Membership.Import.MemberCSV do # Build custom field lookup for efficient value processing custom_field_lookup = build_custom_field_lookup(custom_fields) - # Resolve DB-backed columns (groups, fee types) read-only for the preview. - resolution = ColumnResolver.resolve(maps, rows, actor) - ignored_headers = Enum.map(maps.ignored, &Enum.at(headers, &1)) - {:ok, %{ chunks: chunks, column_map: maps.member, custom_field_map: maps.custom, custom_field_lookup: custom_field_lookup, - warnings: warnings, - headers: headers, - ignored: ignored_headers, - groups_column_index: maps.groups_column_index, - fee_type_column_index: maps.fee_type_column_index, - groups_found: resolution.groups_found, - groups_to_create: resolution.groups_to_create, - fee_type_map: resolution.fee_type_map, - fee_type_warnings: resolution.fee_type_warnings, - has_empty_fee_type_cells?: resolution.has_empty_fee_type_cells?, - preview_rows: resolution.preview_rows + warnings: warnings }} end end @@ -220,7 +180,7 @@ defmodule Mv.Membership.Import.MemberCSV do end) case HeaderMapper.build_maps(headers, custom_field_maps) do - {:ok, %{unknown: unknown} = maps} -> + {:ok, %{member: member_map, custom: custom_map, unknown: unknown}} -> # Build warnings for unknown custom field columns warnings = unknown @@ -237,7 +197,7 @@ defmodule Mv.Membership.Import.MemberCSV do ) end) - {:ok, maps, warnings} + {:ok, %{member: member_map, custom: custom_map}, warnings} {:error, reason} -> {:error, reason} @@ -250,6 +210,8 @@ 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 @@ -290,20 +252,9 @@ defmodule Mv.Membership.Import.MemberCSV do Map.put(acc, custom_field_id, value) end) - %{ - member: member_map, - custom: custom_map, - fee_type: cell_at(row_tuple, tuple_size, maps.fee_type_column_index), - groups: cell_at(row_tuple, tuple_size, maps.groups_column_index) - } + %{member: member_map, custom: custom_map} end - # Returns the raw cell at the given index, or nil if the column is absent. - defp cell_at(_row_tuple, _size, nil), do: nil - - defp cell_at(row_tuple, size, index) when index < size, do: elem(row_tuple, index) - defp cell_at(_row_tuple, _size, _index), do: "" - @doc """ Processes a chunk of CSV rows and creates members. @@ -319,18 +270,12 @@ defmodule Mv.Membership.Import.MemberCSV do - `chunk_rows_with_lines` - List of tuples `{csv_line_number, row_map}` where: - `csv_line_number` - Physical line number in CSV (1-based) - `row_map` - Map with `:member` and `:custom` keys containing field values - - `column_map` - Unused; kept for backward-compatible call sites. Field values are - read from each row's pre-built `:member`/`:custom` maps, not from this argument. - - `custom_field_map` - Unused; kept for backward-compatible call sites (see above). + - `column_map` - Map of canonical field names (atoms) to column indices (for reference) + - `custom_field_map` - Map of custom field IDs (strings) to column indices (for reference) - `opts` - Optional keyword list for processing options: - `:custom_field_lookup` - Map of custom field IDs to metadata (default: `%{}`) - `:existing_error_count` - Number of errors already collected in previous chunks (default: `0`) - `:max_errors` - Maximum number of errors to collect per import overall (default: `50`) - - `:actor` - Actor used for all writes (default: the system actor) - - `:fee_type_map` - Map of normalized fee-type name to fee-type id, used to resolve - each row's fee-type cell (default: `%{}`) - - `:groups_found` - List of pre-fetched `Group` structs seeding in-memory group - resolution; the snapshot grows as groups are auto-created (default: `[]`) ## Error Capping @@ -369,49 +314,27 @@ defmodule Mv.Membership.Import.MemberCSV do existing_error_count = Keyword.get(opts, :existing_error_count, 0) max_errors = Keyword.get(opts, :max_errors, @default_max_errors) actor = Keyword.get(opts, :actor, SystemActor.get_system_actor()) - fee_type_map = Keyword.get(opts, :fee_type_map, %{}) - groups_found = Keyword.get(opts, :groups_found, []) - base_row_opts = %{ - custom_field_lookup: custom_field_lookup, - fee_type_map: fee_type_map, - actor: actor - } - - {inserted, failed, errors, _collected_error_count, truncated?, warnings, groups_acc} = - Enum.reduce(chunk_rows_with_lines, {0, 0, [], 0, false, [], groups_found}, fn {line_number, - row_map}, - {acc_inserted, - acc_failed, - acc_errors, - acc_error_count, - acc_truncated?, - acc_warnings, - acc_groups} -> + {inserted, failed, errors, _collected_error_count, truncated?} = + Enum.reduce(chunk_rows_with_lines, {0, 0, [], 0, false}, fn {line_number, row_map}, + {acc_inserted, acc_failed, + acc_errors, acc_error_count, + acc_truncated?} -> current_error_count = existing_error_count + acc_error_count - row_opts = Map.put(base_row_opts, :groups_found, acc_groups) - case process_row(row_map, line_number, row_opts) do - {:ok, _member, row_warnings, new_groups} -> - {new_inserted, new_failed, new_errors, new_error_count, new_truncated?} = - update_inserted( - {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?} - ) + case process_row(row_map, line_number, custom_field_lookup, actor) do + {:ok, _member} -> + update_inserted( + {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?} + ) - {new_inserted, new_failed, new_errors, new_error_count, new_truncated?, - acc_warnings ++ row_warnings, new_groups} - - {:error, error, new_groups} -> - {new_inserted, new_failed, new_errors, new_error_count, new_truncated?} = - handle_row_error( - {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}, - error, - current_error_count, - max_errors - ) - - {new_inserted, new_failed, new_errors, new_error_count, new_truncated?, acc_warnings, - new_groups} + {:error, error} -> + handle_row_error( + {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}, + error, + current_error_count, + max_errors + ) end end) @@ -420,9 +343,7 @@ defmodule Mv.Membership.Import.MemberCSV do inserted: inserted, failed: failed, errors: Enum.reverse(errors), - errors_truncated?: truncated?, - warnings: warnings, - groups_found: groups_acc + errors_truncated?: truncated? }} end @@ -586,27 +507,18 @@ defmodule Mv.Membership.Import.MemberCSV do defp gettext_error_message(_), do: gettext("Email is invalid.") - # Processes a single row and creates member with custom field values. - # On success returns {:ok, member, warnings, groups}; warnings carry non-fatal - # notices such as an unresolved fee-type name. The returned groups list is the - # accumulated in-memory group snapshot (seeded from the chunk, grown with any - # group created while linking this row) so later rows reuse it instead of - # re-reading the whole Group table per row. + # Processes a single row and creates member with custom field values defp process_row( row_map, line_number, - %{ - custom_field_lookup: custom_field_lookup, - fee_type_map: fee_type_map, - groups_found: groups_found, - actor: actor - } = _row_opts + custom_field_lookup, + actor ) do # Validate row before database insertion case validate_row(row_map, line_number, []) do {:error, error} -> # Return validation error immediately, no DB insert attempted - {:error, error, groups_found} + {:error, error} {:ok, %{member: trimmed_member_attrs, custom: custom_attrs}} -> # Prepare custom field values for Ash @@ -614,119 +526,20 @@ defmodule Mv.Membership.Import.MemberCSV do {:error, validation_errors} -> # Custom field validation errors - return first error first_error = List.first(validation_errors) - - {:error, %Error{csv_line_number: line_number, field: nil, message: first_error}, - groups_found} + {:error, %Error{csv_line_number: line_number, field: nil, message: first_error}} {:ok, custom_field_values} -> - {fee_attrs, warnings} = - resolve_fee_type_attrs(Map.get(row_map, :fee_type), fee_type_map) - - create_member_and_assign_groups( - Map.merge(trimmed_member_attrs, fee_attrs), + create_member_with_custom_fields( + trimmed_member_attrs, custom_field_values, - Map.get(row_map, :groups), - groups_found, line_number, - actor, - warnings + actor ) end end rescue e -> - {:error, %Error{csv_line_number: line_number, field: nil, message: Exception.message(e)}, - groups_found} - end - - # Creates the member, then assigns groups as a post-creation step. A group - # assignment failure fails the row (the member was already created, but the - # row is reported as failed so the operator can act on it). - defp create_member_and_assign_groups( - member_attrs, - custom_field_values, - groups_cell, - groups_found, - line_number, - actor, - warnings - ) do - case create_member_with_custom_fields( - member_attrs, - custom_field_values, - line_number, - actor, - warnings - ) do - {:ok, member, member_warnings} -> - assign_groups(member, groups_cell, groups_found, line_number, actor, member_warnings) - - {:error, error} -> - {:error, error, groups_found} - end - end - - # Assigns the member to all groups listed in the cell, creating missing groups. - # Returns the (possibly grown) group snapshot so the caller can reuse it. - defp assign_groups(member, groups_cell, groups_found, line_number, actor, warnings) do - names = ColumnResolver.split_group_names(groups_cell) - - Enum.reduce_while(names, {:ok, member, warnings, groups_found}, fn name, - {:ok, _m, _w, acc_groups} -> - case link_member_to_group(member, name, acc_groups, actor) do - {:ok, group} -> - {:cont, {:ok, member, warnings, add_group(acc_groups, group)}} - - {:error, reason} -> - {:halt, - {:error, - %Error{ - csv_line_number: line_number, - field: nil, - message: gettext("Group assignment failed: %{reason}", reason: inspect(reason)) - }, acc_groups}} - end - end) - end - - defp add_group(groups, group) do - if Enum.any?(groups, &(&1.id == group.id)), do: groups, else: [group | groups] - end - - defp link_member_to_group(member, name, groups_found, actor) do - with {:ok, group} <- ColumnResolver.create_or_find_group(name, groups_found, actor), - {:ok, _member_group} <- - Mv.Membership.create_member_group( - %{member_id: member.id, group_id: group.id}, - actor: actor - ) do - {:ok, group} - end - end - - # Resolves the fee-type cell into member attrs plus optional warnings. - # Empty cell -> default fee type (SetDefaultMembershipFeeType), no warning. - # Matched name -> membership_fee_type_id attr. - # Unmatched name -> no attr (default applies), warning naming the value. - defp resolve_fee_type_attrs(nil, _fee_type_map), do: {%{}, []} - - defp resolve_fee_type_attrs(cell, fee_type_map) when is_binary(cell) do - trimmed = String.trim(cell) - - if trimmed == "" do - {%{}, []} - else - case Map.get(fee_type_map, ColumnResolver.normalize_fee_type_name(trimmed)) do - nil -> - {%{}, - [ - gettext("Fee type '%{name}' not found; using the default fee type.", name: trimmed) - ]} - - fee_type_id -> - {%{membership_fee_type_id: fee_type_id}, []} - end - end + {:error, %Error{csv_line_number: line_number, field: nil, message: Exception.message(e)}} end # Creates a member with custom field values, handling errors appropriately @@ -734,8 +547,7 @@ defmodule Mv.Membership.Import.MemberCSV do trimmed_member_attrs, custom_field_values, line_number, - actor, - warnings + actor ) do # Convert empty strings to nil for date fields so Ash accepts them member_attrs = sanitize_date_fields(trimmed_member_attrs) @@ -755,7 +567,7 @@ defmodule Mv.Membership.Import.MemberCSV do case Mv.Membership.create_member(final_attrs, actor: actor) do {:ok, member} -> - {:ok, member, warnings} + {:ok, member} {:error, %Ash.Error.Invalid{} = error} -> # Extract email from final_attrs for better error messages diff --git a/lib/mv/membership/member/validations/email_change_permission.ex b/lib/mv/membership/member/validations/email_change_permission.ex index 073da07..2b1c041 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 a98b125..16341c4 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -16,21 +16,6 @@ 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"] @@ -320,7 +305,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()) :: parsed_params() + @spec parse_params(map()) :: map() def parse_params(params) do # DB fields come from "member_fields" raw_member_fields = extract_list(params, "member_fields") @@ -473,6 +458,9 @@ 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) @@ -519,4 +507,6 @@ defmodule Mv.Membership.MemberExport do other -> other end) end + + defp normalize_computed_fields(_), do: [] end diff --git a/lib/mv/membership/members_csv.ex b/lib/mv/membership/members_csv.ex index 0a19810..6331893 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()] | Enumerable.t() + @spec export([struct() | map()], [map()]) :: iodata() 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/members_pdf.ex b/lib/mv/membership/members_pdf.ex index a1c8418..b2989ca 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,6 +211,9 @@ 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 @@ -254,6 +257,8 @@ 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) @@ -316,7 +321,7 @@ defmodule Mv.Membership.MembersPDF do defp format_cell_date_datetime(cell_value, locale) do case DateTime.from_iso8601(cell_value) do - {:ok, datetime, _offset} -> format_datetime(datetime, locale) + {:ok, datetime} -> format_datetime(datetime, locale) _ -> format_cell_date_naive(cell_value, locale) end end diff --git a/lib/mv/membership_fees/cycle_generation_job.ex b/lib/mv/membership_fees/cycle_generation_job.ex index b38886c..71a3158 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, CycleGenerator.results_summary()} | {:error, Ash.Error.t()} + @spec run() :: {:ok, map()} | {:error, term()} 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, CycleGenerator.results_summary()} | {:error, Ash.Error.t()} + @spec run(keyword()) :: {:ok, map()} | {:error, term()} 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, Ash.Error.t()} + @spec pending_members_count() :: {:ok, non_neg_integer()} | {:error, term()} 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()) :: CycleGenerator.generate_result() + @spec run_for_member(String.t()) :: {:ok, [map()]} | {:error, term()} 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 189f40a..8f1bc7c 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -1,11 +1,4 @@ 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. @@ -122,7 +115,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} -> @@ -166,8 +159,7 @@ defmodule Mv.MembershipFees.CycleGenerator do - `{:error, reason}` - Error with reason """ - @spec generate_cycles_for_all_members(keyword()) :: - {:ok, results_summary()} | {:error, Ash.Error.t()} + @spec generate_cycles_for_all_members(keyword()) :: {:ok, map()} | {:error, term()} def generate_cycles_for_all_members(opts \\ []) do today = Keyword.get(opts, :today, Date.utc_today()) batch_size = Keyword.get(opts, :batch_size, 10) @@ -220,7 +212,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/oidc/discovery.ex b/lib/mv/oidc/discovery.ex deleted file mode 100644 index a3a373a..0000000 --- a/lib/mv/oidc/discovery.ex +++ /dev/null @@ -1,88 +0,0 @@ -defmodule Mv.Oidc.Discovery do - @moduledoc """ - Fetches and caches the OIDC provider's discovery document - (`/.well-known/openid-configuration`). - - Currently only `end_session_endpoint` is exposed — used by the logout flow to - trigger RP-initiated logout at the IdP so the user's SSO session is cleared - and they don't get auto-re-logged-in. - - Cache lives in `:persistent_term`, keyed by base URL, for the lifetime of the - BEAM. Re-fetch on next call after `clear_cache/0`. - """ - - require Logger - - @persistent_term_key {__MODULE__, :discovery} - @request_timeout 5_000 - - @doc """ - Returns the IdP's `end_session_endpoint` URL. - - - `{:ok, url}` if discovery succeeds (and is cached for future calls) - - `{:error, reason}` if the IdP is unreachable, the document is malformed, - or the field is missing - """ - @spec end_session_endpoint(String.t()) :: {:ok, String.t()} | {:error, term()} - def end_session_endpoint(base_url) when is_binary(base_url) do - case fetch_cached(base_url) do - {:ok, %{"end_session_endpoint" => url}} when is_binary(url) -> {:ok, url} - {:ok, _config} -> {:error, :no_end_session_endpoint} - {:error, _} = err -> err - end - end - - @doc """ - Clears the cached discovery documents. Intended for tests. - """ - @spec clear_cache() :: :ok - def clear_cache do - :persistent_term.erase(@persistent_term_key) - :ok - end - - @doc """ - Seeds the cache with a fixed result for a base URL. Intended for tests so the - HTTP fetch is skipped. - """ - @spec put_cache(String.t(), {:ok, map()} | {:error, term()}) :: :ok - def put_cache(base_url, result) when is_binary(base_url) do - cache = :persistent_term.get(@persistent_term_key, %{}) - :persistent_term.put(@persistent_term_key, Map.put(cache, base_url, result)) - :ok - end - - defp fetch_cached(base_url) do - cache = :persistent_term.get(@persistent_term_key, %{}) - - case Map.fetch(cache, base_url) do - {:ok, result} -> - result - - :error -> - result = fetch(base_url) - :persistent_term.put(@persistent_term_key, Map.put(cache, base_url, result)) - result - end - end - - defp fetch(base_url) do - url = String.trim_trailing(base_url, "/") <> "/.well-known/openid-configuration" - - case Req.get(url, - receive_timeout: @request_timeout, - connect_options: [timeout: @request_timeout] - ) do - {:ok, %Req.Response{status: 200, body: body}} when is_map(body) -> - {:ok, body} - - {:ok, %Req.Response{status: status}} -> - Logger.warning("OIDC discovery returned HTTP #{status} for #{url}") - {:error, {:http_status, status}} - - {:error, reason} -> - Logger.warning("OIDC discovery request failed for #{url}: #{inspect(reason)}") - {:error, reason} - end - end -end diff --git a/lib/mv/oidc_role_sync.ex b/lib/mv/oidc_role_sync.ex index 0f6467c..a13748a 100644 --- a/lib/mv/oidc_role_sync.ex +++ b/lib/mv/oidc_role_sync.ex @@ -87,6 +87,8 @@ 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/oidc_role_sync_config.ex b/lib/mv/oidc_role_sync_config.ex index bbb5770..2a8574c 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() + Mv.Config.oidc_groups_claim() || "groups" end end diff --git a/lib/mv/release.ex b/lib/mv/release.ex index 5db4751..116b276 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,11 +139,10 @@ 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 @@ -190,16 +189,15 @@ 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}) + 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) |> Ash.update!(authorize?: false) - |> 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) + end) :ok @@ -209,16 +207,15 @@ defmodule Mv.Release do end defp update_admin_user(user, password, admin_role) do - _ = - user - |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) + 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) |> Ash.update!(authorize?: false) - |> 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) + end) :ok end diff --git a/lib/mv/repo.ex b/lib/mv/repo.ex index 183c54f..0a4a04d 100644 --- a/lib/mv/repo.ex +++ b/lib/mv/repo.ex @@ -19,12 +19,4 @@ 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 diff --git a/lib/mv/vereinfacht/client.ex b/lib/mv/vereinfacht/client.ex index 999bd44..3cbba71 100644 --- a/lib/mv/vereinfacht/client.ex +++ b/lib/mv/vereinfacht/client.ex @@ -8,12 +8,6 @@ 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 """ @@ -37,7 +31,7 @@ defmodule Mv.Vereinfacht.Client do {:error, :not_configured} """ @spec test_connection(String.t() | nil, String.t() | nil, String.t() | nil) :: - {:ok, :connected} | {:error, error_reason()} + {:ok, :connected} | {:error, term()} 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} @@ -98,12 +92,13 @@ 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 Mv.Config.sql_sandbox?(), do: [retry: false] ++ opts, else: opts + if @env == :test, do: [retry: false] ++ opts, else: opts end defp post_and_parse_contact(url, body, api_key) do @@ -235,7 +230,7 @@ defmodule Mv.Vereinfacht.Client do Returns the full response body (decoded JSON) for debugging/display. """ - @spec get_contact(String.t()) :: {:ok, map()} | {:error, error_reason()} + @spec get_contact(String.t()) :: {:ok, map()} | {:error, term()} def get_contact(contact_id) when is_binary(contact_id) do fetch_contact(contact_id, []) end diff --git a/lib/mv/vereinfacht/sync_flash.ex b/lib/mv/vereinfacht/sync_flash.ex index 5c643b6..874a717 100644 --- a/lib/mv/vereinfacht/sync_flash.ex +++ b/lib/mv/vereinfacht/sync_flash.ex @@ -37,10 +37,9 @@ 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/vereinfacht/vereinfacht.ex b/lib/mv/vereinfacht/vereinfacht.ex index 4d58f8d..83492b7 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, Mv.Vereinfacht.Client.error_reason()} + @spec test_connection() :: {:ok, :connected} | {:error, term()} def test_connection do Client.test_connection( Mv.Config.vereinfacht_api_url(), diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex index de009b6..d821416 100644 --- a/lib/mv_web/authorization.ex +++ b/lib/mv_web/authorization.ex @@ -113,7 +113,8 @@ defmodule MvWeb.Authorization do iex> can_access_page?(mitglied, "/members") false """ - @spec can_access_page?(map() | nil, String.t()) :: boolean() + @spec can_access_page?(map() | nil, String.t() | Phoenix.VerifiedRoutes.unverified_path()) :: + boolean() def can_access_page?(nil, _page_path), do: false def can_access_page?(user, page_path) do diff --git a/lib/mv_web/components/bulk_actions_dropdown.ex b/lib/mv_web/components/bulk_actions_dropdown.ex deleted file mode 100644 index d0b6172..0000000 --- a/lib/mv_web/components/bulk_actions_dropdown.ex +++ /dev/null @@ -1,243 +0,0 @@ -defmodule MvWeb.Components.BulkActionsDropdown do - @moduledoc """ - Single "Aktionen" dropdown bundling the four member bulk actions, flattened to - one level: open in email program (mailto), copy email addresses, export to CSV, - export to PDF. - - It keeps the CSRF-protected `
` POST export items unchanged (CSV/PDF) and - adds the mailto and copy items that previously lived as standalone header - buttons next to a separate export dropdown. - - ## Scope and trigger badge - - The trigger reads `Aktionen` followed by a scope badge: an emphasized - (`primary`) count `N` when `N` members are selected, and a muted (`neutral`) - badge otherwise — `gefiltert` when a search term or filter narrows the list, - `alle` when nothing is selected and no search/filter is active. Only an actual - selection is emphasized. The badge sits inside the shared `dropdown_menu/1` - trigger via its `trigger_badge` slot, matching the member-filter dropdown's - count badge. The `scope`, `selected_count`, `mailto_bcc`, `recipient_count` - and `mailto_disabled?` are computed by the parent LiveView and passed in. - - ## Recipient handling (mailto / copy) - - The parent already excludes members without an email when building - `mailto_bcc` and `recipient_count` (defensive filter preserved verbatim from - the previous behaviour). Export, by contrast, still includes every member in - scope regardless of email — its payload is unchanged. - - ## Mailto recipient cap - - A mailto URI carries every recipient in its BCC; browsers cannot reliably hand - a very long mailto over to the mail program. When `mailto_disabled?` is true - (recipient count at or above `Mv.Constants.max_mailto_bulk_recipients/0`) the - mailto item is rendered disabled (`aria-disabled`, `tabindex="-1"`, href - dropped) with an explanatory tooltip. Copy and Export have no such cap. - - ## Event routing - - `dropdown_menu/1` sends `toggle_dropdown`/`close_dropdown` to `@myself`, so the - component owns its own `:open` state. The copy item carries an *un-targeted* - `phx-click="copy_emails"`, which therefore reaches the parent LiveView's - `handle_event/3` (which keeps access to `@members`), plus the - `CopyToClipboard` hook. - """ - use MvWeb, :live_component - use Gettext, backend: MvWeb.Gettext - - # Same focus ring as CoreComponents button/dropdown (WCAG 2.4.7) - defp dropdown_item_class do - focus = - MvWeb.CoreComponents.button_focus_classes() - |> Kernel.++(["focus-visible:ring-inset"]) - |> Enum.join(" ") - - "flex items-center gap-2 px-2 py-1 rounded cursor-pointer hover:bg-base-200 w-full text-left whitespace-nowrap #{focus}" - end - - @impl true - def mount(socket) do - {:ok, assign(socket, :open, false)} - end - - @impl true - def update(assigns, socket) do - socket = - socket - |> assign(:id, assigns.id) - |> assign(:export_payload_json, assigns[:export_payload_json] || "") - |> assign(:selected_count, assigns[:selected_count] || 0) - |> assign(:scope, assigns[:scope] || :all) - |> assign(:mailto_bcc, assigns[:mailto_bcc] || "") - |> assign(:recipient_count, assigns[:recipient_count] || 0) - |> assign(:mailto_disabled?, assigns[:mailto_disabled?] || false) - - # The parent never sets :open (the component owns it via toggle/close). - # Honouring an explicit :open assign keeps the component renderable in - # isolation (render_component/2) for structural tests. - socket = - case Map.fetch(assigns, :open) do - {:ok, open} -> assign(socket, :open, open) - :error -> socket - end - - {:ok, socket} - end - - @impl true - def render(assigns) do - assigns = - assigns - |> assign(:scope_label, scope_label(assigns)) - |> assign(:scope_variant, scope_variant(assigns)) - - ~H""" -
- <.dropdown_menu - id={"#{@id}-menu"} - button_label={gettext("Actions")} - icon="hero-bolt" - open={@open} - phx_target={@myself} - menu_width="w-70" - menu_align="left" - button_class="btn-secondary gap-2" - testid="bulk-actions-dropdown" - button_testid="bulk-actions-button" - menu_testid="bulk-actions-menu" - > - <:trigger_badge> - <.badge variant={@scope_variant} size="sm" data-testid="bulk-actions-scope-badge"> - {@scope_label} - - -
  • - <.mailto_item mailto_bcc={@mailto_bcc} disabled={@mailto_disabled?} /> -
    • -
  • - -
    • -
  • - - - - -
    • - -
  • -
    - - - -
    -
    • - -
    - """ - end - - # The mailto item is an anchor menu item. When over the recipient cap it is - # rendered disabled following the same a11y pattern as a disabled CoreComponents - # link button (href dropped, tabindex=-1, aria-disabled=true) and exposes the - # explanatory tooltip via title. - attr :mailto_bcc, :string, required: true - attr :disabled, :boolean, required: true - - defp mailto_item(%{disabled: true} = assigns) do - assigns = assign(assigns, :item_class, dropdown_item_class()) - - ~H""" - - <.icon name="hero-envelope" class="h-4 w-4" /> - {gettext("Open in email program")} - - """ - end - - defp mailto_item(%{disabled: false} = assigns) do - assigns = assign(assigns, :item_class, dropdown_item_class()) - - ~H""" - @mailto_bcc} - class={@item_class} - aria-label={gettext("Open in email program")} - data-testid="bulk-actions-mailto" - > - <.icon name="hero-envelope" class="h-4 w-4" /> - {gettext("Open in email program")} - - """ - end - - defp over_threshold_tooltip do - gettext("Too many recipients for this function. Copy the addresses or export the list.") - end - - # The trigger scope is shown as a badge after the "Aktionen" label. Only an - # actual selection is emphasized (primary); both the "filtered" and "all" - # scopes are muted (neutral), since neither means members are selected. - defp scope_label(assigns) do - case assigns.scope do - :selection -> to_string(assigns.selected_count) - :filtered -> gettext("filtered") - _ -> gettext("all") - end - end - - defp scope_variant(assigns) do - case assigns.scope do - :selection -> "primary" - _ -> "neutral" - end - end - - @impl true - def handle_event("toggle_dropdown", _params, socket) do - {:noreply, assign(socket, :open, !socket.assigns.open)} - end - - def handle_event("close_dropdown", _params, socket) do - {:noreply, assign(socket, :open, false)} - end -end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 13c69a8..465d41a 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -464,9 +464,6 @@ defmodule MvWeb.CoreComponents do slot :inner_block, doc: "Custom content for the dropdown menu (e.g., forms)" - slot :trigger_badge, - doc: "Optional badge rendered in the trigger after the label (e.g. a scope badge)" - def dropdown_menu(assigns) do menu_testid = assigns.menu_testid || "#{assigns.testid}-menu" @@ -501,8 +498,6 @@ defmodule MvWeb.CoreComponents do <.icon name={@icon} /> <% end %> {@button_label} - {render_slot(@trigger_badge)} - <.icon name="hero-chevron-down" class="size-4" />