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..9f18072 --- /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.59 + 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..e24b118 100644 --- a/.env.example +++ b/.env.example @@ -14,7 +14,6 @@ ASSOCIATION_NAME="Sportsclub XYZ" # Optional: Admin user (created/updated on container start via Release.seed_admin) # In production, set these so the first admin can log in. Change password without redeploy: # bin/mv eval "Mv.Release.seed_admin()" (with new ADMIN_PASSWORD or ADMIN_PASSWORD_FILE) -# FORCE_SEEDS=true re-runs bootstrap seeds even when admin user exists (e.g. after changing roles/custom fields). # ADMIN_EMAIL=admin@example.com # ADMIN_PASSWORD=secure-password # ADMIN_PASSWORD_FILE=/run/secrets/admin_password @@ -24,7 +23,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. @@ -42,15 +41,3 @@ ASSOCIATION_NAME="Sportsclub XYZ" # VEREINFACHT_API_KEY=your-api-key # VEREINFACHT_CLUB_ID=2 # VEREINFACHT_APP_URL=https://app.verein.visuel.dev - -# Optional: Mail / SMTP (transactional emails). If set, overrides Settings UI. -# Export current UI settings to .env: mix mv.export_smtp_to_env -# SMTP_HOST=smtp.example.com -# SMTP_PORT=587 -# SMTP_USERNAME=user -# SMTP_PASSWORD=secret -# SMTP_PASSWORD_FILE=/run/secrets/smtp_password -# SMTP_SSL=tls -# SMTP_VERIFY_PEER=false -# MAIL_FROM_EMAIL=noreply@example.com -# MAIL_FROM_NAME=Mila 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..275206c 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.46.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index adbe7e7..2c23c01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,87 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [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 -- **Clickable table row highlights** – The new hover/focus-visible row highlight behavior is now the CoreComponents default across clickable tables. Sticky-first-column tables keep zebra striping and show selection through the sticky-column accent stripe (checkboxes keep their default style). -- **Members overview scrolling** – The members table scrollbar now scrolls inside the table container instead of moving with the full page. -- **Join request display and settings workflow** – Improved join request rendering and related settings behavior in one cohesive update: - - Join request fields now respect their configured field types in the details view. - - Custom field labels in join request views were standardized. - - Join request field formatting was corrected for more consistent output. - - Join link settings now include a direct "Open" action in addition to copy/share workflows. - -### Fixed -- **Runtime ENV handling** – Empty or invalid environment variables (e.g. `SMTP_PORT=`, `PORT=`, `POOL_SIZE=`, `DATABASE_PORT=`) no longer cause `ArgumentError` at boot. Instead raises clear errors for required vars set but empty (e.g. DATABASE_HOST, PHX_HOST/DOMAIN, SECRET_KEY_BASE). -- **PostgreSQL 18 Docker volume path** – Corrected the database volume path to match PostgreSQL 18 expectations. -- **Association name ENV handling** – `ASSOCIATION_NAME` is now treated as source of truth; the field is read-only in Global Settings when managed via ENV. -- **Association name consistency after updates** – Layout now prefers explicitly assigned `club_name` values to avoid stale cached values right after settings changes. -- **SMTP ENV/UI source selection** – SMTP now follows a strict single-source policy: ENV-only when `SMTP_HOST` is set, otherwise Settings-only. -- **SMTP settings UI in ENV mode** – SMTP fields are read-only, save action is hidden, and missing required ENV keys are shown as a warning. - -### Dependency updates -- Mix dependencies were updated. -- Renovate Docker image was updated to `v43.165`. -- Rauthy Docker image was updated to `v0.35.1`. -- `just` was updated to `v1.50.0`. - -## [1.1.1] - 2026-03-16 - -### Added -- **FORCE_SEEDS** – Environment variable. When set to `"true"`, bootstrap (and optionally dev) seeds are run even when the admin user already exists, so you can re-apply changed seed data (e.g. new roles or custom fields) without deleting the admin user. -- **Improved OIDC-only mode** – Admin can enable “Only OIDC sign-in” in settings; when enabled, direct registration is disabled and sign-in page redirects to OIDC when configured. -- **Success toast auto-dismiss** – Success flash messages (e.g. “Settings saved”) hide automatically after 5 seconds instead of requiring the user to close them. - -### Changed -- **Seeds run only when needed** – Bootstrap and dev seeds are skipped on application start when the admin user already exists (`Mv.Release.bootstrap_seeds_applied?/0`). This avoids duplicate data and speeds up startup in dev and production after the first run. Set `FORCE_SEEDS=true` to override and re-run. -- **Unauthenticated access** – Users who are not logged in are redirected to sign-in without showing a “no permission” message; the message is only shown to logged-in users who lack access. - -### Fixed -- **SMTP configuration** – Repaired so that both port 587 (TLS/STARTTLS) and 465 (SSL) work correctly. - -## [1.1.0] - 2026-03-13 - -### Added -- **Browser timezone for datetime display** – Date/time values (e.g. join request submitted at, approved at, rejected at) are shown in the user’s local timezone. -- **Registration toggle** – New global setting to disable direct registration (`/register`). When disabled, visitors are redirected to sign-in and the register link is hidden; join form remains available. -- **Configurable SMTP in global settings** – SMTP host, port, user, password, and TLS options configurable via Admin → Global Settings. Test-email action to verify delivery. Join confirmation and other transactional emails use this configuration. -- **Theme and language selector on unauthenticated pages** – Sign-in and join pages now offer theme (light/dark) and locale (e.g. German/English) controls in the header. -- **Duplicate-email handling for join form** – If an applicant’s email is already a member or already has a pending join request, the system sends a clarifying email (already-member or already-pending) and shows the same success message (anti-enumeration). -- **Reviewed-by display for join requests** – Approval UI shows who reviewed a request via a dedicated display field, without loading the User record. -- **Improved field order and seeds for join request approval** – Approval screen field order improved; seed data updated for join-form and approval flows. -- **Tests for SMTP mailer configuration** – Tests for SMTP config and for join confirmation email delivery failure (domain and LiveView). - -### Changed -- **SMTP settings layout** – SMTP options reordered and grouped in global settings for clearer configuration. -- **Join confirmation mail** – Uses configurable SMTP from settings; on delivery failure the join form shows an error and no success message. -- **i18n** – Gettext catalogs updated for new and changed strings. - -### Fixed -- **Login page translation** – Corrected translation/locale handling on the sign-in page. - ---- - -## [1.0.0] and earlier - ### Added - **Roles and Permissions System (RBAC)** - Complete implementation (#345, 2026-01-08) - Four hardcoded permission sets: `own_data`, `read_only`, `normal_user`, `admin` diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index ccd16f4..0cb8d65 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -90,8 +90,6 @@ lib/ │ ├── custom_field.ex # Custom field (definition) resource │ ├── custom_field_value.ex # Custom field value resource │ ├── setting.ex # Global settings (singleton resource; incl. join form config) -│ ├── settings_cache.ex # Process cache for get_settings (TTL; invalidate on update; not started in test) -│ ├── join_notifier.ex # Behaviour for join emails (confirmation, already member, already pending) │ ├── setting/ # Setting changes (NormalizeJoinFormSettings, etc.) │ ├── group.ex # Group resource │ ├── member_group.ex # MemberGroup join table resource @@ -130,8 +128,6 @@ lib/ │ ├── constants.ex # Application constants (member_fields, custom_field_prefix, vereinfacht_required_member_fields) │ ├── application.ex # OTP application │ ├── mailer.ex # Email mailer -│ ├── smtp/ -│ │ └── config_builder.ex # SMTP adapter opts (TLS/sockopts); used by runtime.exs and Mailer │ ├── release.ex # Release tasks │ ├── repo.ex # Database repository │ ├── secrets.ex # Secret management @@ -284,13 +280,13 @@ end ### 1.2.1 Database Seeds -Seeds are split into **bootstrap** and **dev**. They run on every start (e.g. `just run`, Docker entrypoint) but **exit early** if already applied so startup stays fast and no duplicate data is created. +Seeds are split into **bootstrap** and **dev**: -- **`priv/repo/seeds.exs`** – Entrypoint. If the admin user (ADMIN_EMAIL or default) already exists, skips entirely (unless `FORCE_SEEDS=true`); otherwise runs `seeds_bootstrap.exs` and, in dev/test, `seeds_dev.exs`. +- **`priv/repo/seeds.exs`** – Entrypoint. Runs `seeds_bootstrap.exs` always; runs `seeds_dev.exs` only when `Mix.env()` is `:dev` or `:test`. - **`priv/repo/seeds_bootstrap.exs`** – Creates only data required for system startup: membership fee types, custom fields, roles, admin user, system user, global settings (including default membership fee type). No members, no groups. Used in all environments (dev, test, prod). - **`priv/repo/seeds_dev.exs`** – Creates 20 sample members, groups, and optional custom field values. Run only in dev and test. -In production, running `mix run priv/repo/seeds.exs` (or `Mv.Release.run_seeds/0`) executes only the bootstrap part when not yet applied (no dev seeds unless `RUN_DEV_SEEDS=true`). The “already applied” check uses `Mv.Release.bootstrap_seeds_applied?/0` (admin user exists). Set `FORCE_SEEDS=true` to re-run seeds even when already applied. +In production, running `mix run priv/repo/seeds.exs` executes only the bootstrap part (no dev seeds). ### 1.3 Domain-Driven Design @@ -1277,17 +1273,14 @@ mix hex.outdated **SMTP configuration:** -- SMTP can be configured via **ENV variables** (`SMTP_HOST`, `SMTP_PORT`, `SMTP_USERNAME`, `SMTP_PASSWORD`, `SMTP_PASSWORD_FILE`, `SMTP_SSL`) or via **Admin Settings** (database: `smtp_host`, `smtp_port`, `smtp_username`, `smtp_password`, `smtp_ssl`). -- **ENV-only policy:** If `SMTP_HOST` is set, SMTP is treated as environment-managed only. All SMTP fields in Settings are read-only, SMTP save action is hidden, and the UI shows a warning when required ENV values are missing (`SMTP_USERNAME`, and `SMTP_PASSWORD` or `SMTP_PASSWORD_FILE`). This keeps one source of truth for transport credentials and avoids mixed ENV/DB SMTP states. +- SMTP can be configured via **ENV variables** (`SMTP_HOST`, `SMTP_PORT`, `SMTP_USERNAME`, `SMTP_PASSWORD`, `SMTP_PASSWORD_FILE`, `SMTP_SSL`) or via **Admin Settings** (database: `smtp_host`, `smtp_port`, `smtp_username`, `smtp_password`, `smtp_ssl`). ENV takes priority (same pattern as OIDC/Vereinfacht). - **Sensitive settings in DB:** `smtp_password` and `oidc_client_secret` are excluded from the default read of the Setting resource; they are loaded only via explicit select when needed (e.g. `Mv.Config.smtp_password/0`, `Mv.Config.oidc_client_secret/0`). This avoids exposing secrets through `get_settings()`. -- **Settings cache:** `Mv.Membership.get_settings/0` uses `Mv.Membership.SettingsCache` when the cache process is running (not in test). Cache has a short TTL and is invalidated on every settings update. This avoids repeated DB reads on hot paths (e.g. `RegistrationEnabled` validation, `Layouts.public_page`). In test, the cache is not started so all callers use `get_settings_uncached/0` in the test process (Ecto Sandbox). -- **Join emails (domain → web):** The domain calls `Mv.Membership.JoinNotifier` (config `:join_notifier`, default `MvWeb.JoinNotifierImpl`) for sending join confirmation, already-member, and already-pending emails. This keeps the domain independent of the web layer; tests can override the notifier. - Sender identity is also configurable via ENV (`MAIL_FROM_NAME`, `MAIL_FROM_EMAIL`) or Settings (`smtp_from_name`, `smtp_from_email`). - `SMTP_PASSWORD_FILE`: path to a file containing the password (Docker Secrets / Kubernetes secrets pattern); overridden by `SMTP_PASSWORD` when both are set. - `SMTP_SSL` values: `tls` (default, port 587), `ssl` (port 465), `none` (port 25). - When `SMTP_HOST` ENV is present at boot, `runtime.exs` configures `Swoosh.Adapters.SMTP` automatically. - When SMTP is configured only via Settings, `Mv.Mailer.smtp_config/0` builds the adapter config per-send. -- In test environment, `Swoosh.Adapters.Test` is used regardless of SMTP config. `smtp_config/0` returns `[]` when the mailer adapter is `Swoosh.Adapters.Test`, so per-send SMTP opts never bypass the test mailbox. Port 587/465 sockopts are unit-tested on `Mv.Smtp.ConfigBuilder.build_opts/1` (`test/mv/smtp/config_builder_test.exs`); `test/mv/mailer_smtp_config_test.exs` covers the Test-adapter guard and temporarily sets the adapter to `Swoosh.Adapters.Local` to assert `smtp_config/0` wiring from ENV. Use `Mv.DataCase` for those tests (not plain `ExUnit.Case`) because `smtp_config/0` pulls `Mv.Config` fields that may read Settings from the DB when SMTP user/password ENV vars are unset. +- In test environment, `Swoosh.Adapters.Test` is used regardless of SMTP config. - **TLS in OTP 27:** Verify mode defaults to `verify_none` for self-signed/internal certs. Set `SMTP_VERIFY_PEER=true` (or `1`/`yes`) in prod when using public SMTP (Gmail, Mailgun). Config key `:smtp_verify_peer` is set in `runtime.exs` and read by `Mv.Mailer.smtp_config/0`. - **Test email:** `Mv.Mailer.send_test_email(to_email)` sends a transactional test email; returns `{:ok, email}` or `{:error, classified_reason}`. Classified errors: `:sender_rejected`, `:auth_failed`, `:recipient_rejected`, `:tls_failed`, `:connection_failed`, `{:smtp_error, message}`. Each shows a specific message in the UI. - **Production warning:** When SMTP is not configured in production, a warning is shown in the Settings UI. Use `Application.get_env(:mv, :environment, :dev)` (or assign in mount) for environment checks in LiveView/templates; do not use `Mix.env()` at runtime (it is not available in releases). @@ -1297,10 +1290,6 @@ mix hex.outdated - `SendPasswordResetEmail` and `SendNewUserConfirmationEmail` use `Mv.Mailer.deliver/1` (not `deliver!/1`). Errors are logged via `Logger.error` and not re-raised so they never crash the caller process. -**Join confirmation email:** - -- Join emails are sent via `Mv.Membership.JoinNotifier` (default impl: `MvWeb.JoinNotifierImpl` calling `JoinConfirmationEmail`, etc.). `MvWeb.Emails.JoinConfirmationEmail` uses `Mailer.deliver(email, Mailer.smtp_config())` so it uses the same SMTP configuration as the test mail (Settings or boot ENV). On delivery failure, `Mv.Membership.submit_join_request/2` returns `{:error, :email_delivery_failed}` (and logs via `Logger.error`); the JoinLive shows an error message and no success UI. - **Unified layout (transactional emails):** - All transactional emails (join confirmation, user confirmation, password reset) use the same layout: `MvWeb.EmailLayoutView` (layout) and `MvWeb.EmailsView` (body templates). @@ -1363,8 +1352,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 @@ -1715,8 +1702,6 @@ mix test test/membership/member_test.exs:42 ### 4.7 Testing Best Practices -**Process environment (`test/test_helper.exs`):** Vereinfacht and OIDC-related `System.get_env/1` keys are cleared at test startup so configuration comes from the test database (Membership settings) unless a test explicitly sets variables in `setup` and restores them with `on_exit`. This matches production priority (ENV over settings) while keeping the suite deterministic when `.env` is loaded (e.g. via `just`). - **Testing Philosophy: Focus on Business Logic, Not Framework Functionality** We test our business logic and domain-specific behavior, not core framework features. Framework features (Ash validations, Ecto relationships, etc.) are already tested by their respective libraries. @@ -2182,14 +2167,6 @@ mix phx.gen.secret mix phx.gen.secret ``` -**Runtime configuration (config/runtime.exs):** - -- Production config is loaded from `config/runtime.exs` at boot (releases and `mix phx.server`). Environment variables are read via helpers so that **empty or invalid values do not cause cryptic crashes** (e.g. `ArgumentError` from `String.to_integer("")`). -- **Helpers used:** `get_env_or_file` / `get_env_or_file!` (with `_FILE` support); `get_env_required` (required vars: raises if missing or empty after trim); `get_env_non_empty` (optional string: empty treated as unset, returns default); `parse_positive_integer` (PORT, POOL_SIZE, SMTP_PORT: empty or invalid → default). -- **Required vars** (e.g. DATABASE_HOST, PHX_HOST/DOMAIN, SECRET_KEY_BASE): if set but empty, the app raises at boot with a clear message including “(Variable X is set but empty.)”. -- **Optional numeric vars** (PORT, POOL_SIZE, SMTP_PORT, DATABASE_PORT): empty or invalid value is treated as “unset” and the documented default is used (e.g. PORT=4000, SMTP_PORT=587). -- When adding new ENV in `runtime.exs`, use these helpers instead of raw `System.get_env(...)` and `String.to_integer(...)` so that misconfigured or empty variables fail fast with clear errors. - ### 5.6 Security Headers **Configure Security Headers:** diff --git a/DESIGN_GUIDELINES.md b/DESIGN_GUIDELINES.md index 34c71b8..0a10a70 100644 --- a/DESIGN_GUIDELINES.md +++ b/DESIGN_GUIDELINES.md @@ -46,14 +46,14 @@ Every authenticated page should follow the same structure: **MUST:** Use `<.header>` on every page (except login/public pages). **SHOULD:** Put short explanations into `<:subtitle>` rather than sprinkling random text blocks. -### 2.2 Edit/New form header: Back button left (mandatory) +### 2.2 Edit/New form header and footer buttons (mandatory) For LiveViews that render an edit or new form (e.g. member, group, role, user, custom field, membership fee type): -- **MUST:** Provide a **Back** button on the **left** side of the header using the `<:leading>` slot (same as data fields: Back left, title next, primary action on the right). +- **MUST:** Provide a **Back** button on the **left** side of the header using the `<:leading>` slot (same as data fields: Back left, title next). - **MUST:** Use the same pattern everywhere: Back button with `variant="neutral"`, arrow-left icon, and label “Back”. It navigates to the previous context (e.g. detail page or index) via a `return_path`-style helper. -- **SHOULD:** Place the primary action (e.g. “Save”) in `<:actions>` on the right. -- **Rationale:** Users expect a consistent way to leave the form without submitting; Back left matches the data fields edit view and keeps primary actions on the right. +- **MUST:** Place **exactly one** form button bar **below all form fields**, inside the `<.form>`, with: **Abbrechen** (Cancel) left, **Speichern** (Save) right. Use `gettext("Cancel")`, `gettext("Save ")`, `phx-disable-with={gettext("Saving...")}` on the submit button. No submit button in the header; no duplicate submit buttons. +- **Rationale:** Users expect a consistent way to leave the form without submitting; Back left. One primary action (Save) per form, in the footer, avoids double submits and matches the reference (member edit form). **Template for form pages:** ```heex @@ -66,31 +66,21 @@ For LiveViews that render an edit or new form (e.g. member, group, role, user, c Page title (e.g. “Edit Member” or “New User”) <:subtitle>Short explanation. - <:actions> - <.button phx-disable-with={gettext("Saving...")} variant="primary" type="submit"> - {gettext("Save")} - - + +<.form for={@form} id="..." phx-change="validate" phx-submit="save"> + <%!-- form sections and fields --%> +
+ <.button navigate={return_path(@return_to, @resource)} variant="neutral" type="button"> + {gettext("Abbrechen")} + + <.button type="submit" phx-disable-with={gettext("Speichern...")} variant="primary"> + {gettext("Speichern")} + +
+ ``` -If the `<.header>` is outside the `<.form>`, the submit button must reference the form via the `form` attribute (e.g. `form="user-form"`). - -### 2.3 Public / unauthenticated pages (Join, Sign-in, Join Confirm) - -Pages that do not require authentication (e.g. `/join`, `/sign-in`, `/confirm_join/:token`) use a unified layout via the **`Layouts.public_page`** component: - -- **Component:** `Layouts.public_page` renders: - - **Header:** Logo + "Mitgliederverwaltung" (left) | Club name centered via absolute positioning | Language selector + theme swap (sun/moon, DaisyUI swap with rotate) (right) - - Main content slot, Flash group. No sidebar, no authenticated-layout logic. -- **Content:** DaisyUI **hero** section (`hero`, `hero-content`) for the main message or form, so all public pages share the same visual structure. The hero is constrained in width (`max-w-4xl mx-auto`) and content is left-aligned (`hero-content flex-col items-start text-left`). -- **Locale handling:** The language selector uses `Gettext.get_locale(MvWeb.Gettext)` (backend-specific) to correctly reflect the active locale. `SignInLive` sets both `Gettext.put_locale(MvWeb.Gettext, locale)` and `Gettext.put_locale(locale)` to keep global and backend locales in sync. -- **Translations for AshAuthentication components:** AshAuthentication’s `_gettext` mechanism translates button labels (e.g. “Sign in” → “Anmelden”, “Register” → “Registrieren”) at runtime via `gettext_fn: {MvWeb.Gettext, "auth"}`. Components that do NOT use `_gettext` (e.g. `HorizontalRule`) receive static German overrides via **`MvWeb.AuthOverridesDE`**, which is prepended to the overrides list in `SignInLive` when the locale is `"de"`. -- **Implementation:** - - **Sign-in** (`SignInLive`): Uses `use Phoenix.LiveView` (not `use MvWeb, :live_view`) so AshAuthentication’s sign_in_route live_session on_mount chain is not mixed with LiveHelpers hooks. Renders `` with the SignIn component inside a hero. Displays a locale-aware `

` title (“Anmelden” / “Registrieren”) above the AshAuthentication component (the library’s Banner is hidden via `show_banner: false`). - - **Join** (`JoinLive`): Uses `use MvWeb, :live_view` and wraps content in `` with a hero for the form. - - **Join Confirm** (controller): Uses `JoinConfirmHTML` with a template that wraps content in `` and a hero block for the result, so the confirm page shares the same header and chrome as Join and Sign-in. - ## 3) Typography (system) Use these standard roles: @@ -98,18 +88,16 @@ Use these standard roles: | Role | Use | Class | |---|---|---| | Page title (H1) | main page title | `text-xl font-semibold leading-8` | -| Subtitle | helper under title | `text-sm text-base-content/85` | +| Subtitle | helper under title | `text-sm text-base-content/70` | | Section title (H2) | section headings | `text-lg font-semibold` | -| Helper text | under inputs | `text-sm text-base-content/85` | -| Fine print | small hints | `text-xs text-base-content/80` | -| Empty state | no data | `text-base-content/80 italic` | +| Helper text | under inputs | `text-sm text-base-content/70` | +| Fine print | small hints | `text-xs text-base-content/60` | +| Empty state | no data | `text-base-content/60 italic` | | Destructive text | danger | `text-error` | **MUST:** Page titles via `<.header>`. **MUST:** Section titles via `<.form_section title="…">` (for forms) or a consistent section wrapper (if you introduce a `<.card>` later). -**Form labels (WCAG 2.2 AA):** DaisyUI `.label` defaults to 60% opacity and fails contrast. We override it in `app.css` to 85% of `base-content` so labels stay slightly de‑emphasised vs body text but meet the 4.5:1 minimum. Use `class="label"` and `` as usual; no extra classes needed. - --- ## 4) States: Loading, Empty, Error (mandatory consistency) @@ -221,11 +209,6 @@ If these cannot be met, use `secondary`/`outline` instead of `ghost`. - **MUST:** Required fields are marked consistently (UI indicator + accessible text). - **SHOULD:** If required-ness is configurable via settings, display it consistently in the form. -### 6.4 Form layout (settings / long forms) -- **SHOULD:** On wide viewports, use a responsive grid so related fields share a row and reduce scrolling (e.g. `grid grid-cols-1 lg:grid-cols-2` or `lg:grid-cols-[2fr_5rem_1fr]` for mixed widths). -- **SHOULD:** Limit the main content width for readability (e.g. Settings page uses `max-w-4xl mx-auto px-4` around the content area below the header). -- **Example:** SMTP settings use three rows on large screens (Host, Port, TLS/SSL | Username, Password | Sender email, Sender name) without subsection labels. - --- ## 7) Lists, Search & Filters (mandatory UX consistency) @@ -247,13 +230,11 @@ If these cannot be met, use `secondary`/`outline` instead of `ghost`. ### 8.1 Default behavior: row click opens details - **DEFAULT:** Clicking a row navigates to the details page. - **EXCEPTIONS:** Highly interactive rows may disable row-click (document why). -- **Row highlight (CoreComponents):** When `row_click` is set, rows use a neutral background highlight on `hover` and `tr:has(:focus-visible)` (see `assets/css/app.css`), so keyboard focus is visible while mouse-only focus does not appear "stuck". For non-sticky tables, `selected_row_id` can still add a stronger selected ring. For sticky-first-column tables, selection emphasis is handled by the sticky-column accent stripe. +- **Row outline (CoreComponents):** When `row_click` is set, rows get a subtle hover and focus-within ring (theme-friendly). Use `selected_row_id` to show a stronger selected outline (e.g. from URL `?highlight=id` or last selection); the Back link from detail can use `?highlight=id` so the row is visually selected when returning to the index. **IMPORTANT (correctness with our `<.table>` CoreComponent):** Our table implementation attaches the `phx-click` to the **``** when `row_click` is set. That means click events bubble from inner elements up to the cell unless we stop propagation. -**LiveStream rows:** Do not enumerate `@rows` with `Enum.with_index` in the table template; streams must be consumed only through `:for`. Sticky-first-column zebra striping for those tables is handled in CSS (`nth-child` under `data-sticky-first-col-rows`), not by assigning odd/even classes from an index. - So, for interactive elements inside a clickable row, you must **stop propagation using `Phoenix.LiveView.JS.stop_propagation/1`**, not a custom attribute. ✅ Correct pattern (one click handler that both stops propagation and triggers an event): diff --git a/Justfile b/Justfile index 9b0be65..f3ad5a3 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 @@ -16,7 +10,6 @@ install-dependencies: mix deps.get migrate-database: - mix compile mix ash.setup reset-database: @@ -29,27 +22,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 +36,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 +68,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..92b15d9 100644 --- a/README.md +++ b/README.md @@ -106,9 +106,6 @@ export PATH="${ASDF_DATA_DIR:-$HOME/.asdf}/shims:$PATH" ```bash git clone https://git.local-it.org/local-it/mitgliederverwaltung.git mila cd mila -asdf plugin add elixir -asdf plugin add erlang -asdf plugin add just asdf install # Inside the repo folder: @@ -124,8 +121,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 +136,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/assets/css/app.css b/assets/css/app.css index 611e9ad..4118f09 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -154,14 +154,6 @@ background-color: var(--color-base-100); } -/* WCAG 2.2 AA (4.5:1 for normal text): Form labels. DaisyUI .label uses 60% opacity, - which fails contrast. Override to 85% of base-content so labels stay slightly - de‑emphasised vs body text but meet the minimum ratio. Match .label directly - so the override applies even when data-theme is not yet set (e.g. initial load). */ -.label { - color: color-mix(in oklab, var(--color-base-content) 85%, transparent); -} - /* WCAG 2.2 AA (4.5:1 for normal text): Badge text must contrast with badge background. Theme tokens *-content are often too light on * backgrounds in light theme, and badge-soft uses variant as text on a light tint (low contrast). We override @@ -585,7 +577,9 @@ } /* ============================================ - WCAG 2.2 AA: Tab list inactive tab text contrast (4.5:1) + Member detail tabs (show + edit): inactive vs active contrast + WCAG 2.2 AA: inactive tab text contrast (4.5:1) + Active tab: visible border (DaisyUI tabs-bordered) and weight so which tab is selected is clear. ============================================ */ #member-tablist .tab:not(.tab-active) { color: oklch(0.35 0.02 285); @@ -594,6 +588,13 @@ color: oklch(0.72 0.02 257); } +/* Active tab: stronger underline (DaisyUI --tab-border-color) and font weight */ +#member-tablist .tab.tab-active, +#member-tablist .tab[aria-selected="true"] { + --tab-border-color: var(--color-base-content); + font-weight: 600; +} + /* ============================================ WCAG 2.2 AA: Link contrast - primary and accent ============================================ */ @@ -708,68 +709,3 @@ background-color: transparent !important; color: inherit; } - -/* - * Default interactive table rows: neutral hover/focus-visible fill for clickable rows. - * Uses :has(:focus-visible) so keyboard navigation highlights the row without sticky mouse-focus artifacts. - */ -.table.table-zebra tbody tr[data-row-interactive="true"]:is(:hover, :has(:focus-visible)) > td { - background-color: var(--color-base-300); -} - -/* - * Sticky first column in zebra tables: opaque backgrounds per row. - * Use nth-child (not HEEx row index) so LiveStream rows stay iterable only via :for (Phoenix LV requirement). - */ -[data-sticky-first-col-rows="true"] .table.table-zebra tbody tr:nth-child(odd) > td.sticky-first-col-cell { - background-color: var(--color-base-100); -} - -[data-sticky-first-col-rows="true"] .table.table-zebra tbody tr:nth-child(even) > td.sticky-first-col-cell { - background-color: var(--color-base-200); -} - -/* - * Checkbox-selected rows: keep zebra backgrounds; only accent the sticky checkbox column. - */ -[data-sticky-first-col-rows="true"] - .table.table-zebra - tbody - tr[data-selected="true"] - > td.sticky-first-col-cell { - box-shadow: inset 2px 0 0 var(--color-primary); -} - -[data-sticky-first-col-rows="true"] - .table.table-zebra - tbody - tr[data-row-interactive="true"]:is(:hover, :has(:focus-visible)) - > td.sticky-first-col-cell { - background-color: var(--color-base-300); - /* Left accent only; keep the familiar orange primary accent. */ - box-shadow: inset 2px 0 0 var(--color-primary); -} - -/* - * Sticky member selection table: drop mouse-only focus outlines that read like a thin frame around the row; - * keyboard :focus-visible keeps DaisyUI control outlines (checkbox / tabindex cell). - */ -[data-sticky-first-col-rows="true"] .table.table-zebra tbody tr { - outline: none; -} - -[data-sticky-first-col-rows="true"] - .table.table-zebra - tbody - tr[data-row-interactive="true"]:is(:hover, :has(:focus-visible)):not(:last-child) { - /* DaisyUI draws a bottom border on each row; hiding it while highlighted avoids a boxy “frame”. */ - border-bottom-color: transparent; -} - -[data-sticky-first-col-rows="true"] .table.table-zebra tbody tr td:focus:not(:focus-visible) { - outline: none; -} - -[data-sticky-first-col-rows="true"] .table.table-zebra tbody tr input.checkbox:focus:not(:focus-visible) { - outline: none; -} diff --git a/assets/js/app.js b/assets/js/app.js index 4c7e3c5..ee423eb 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -25,14 +25,6 @@ import Sortable from "../vendor/sortable" let csrfToken = document.querySelector("meta[name='csrf-token']").getAttribute("content") -function getBrowserTimezone() { - try { - return Intl.DateTimeFormat().resolvedOptions().timeZone || null - } catch (_e) { - return null - } -} - // Hooks for LiveView components let Hooks = {} @@ -113,25 +105,6 @@ Hooks.FocusRestore = { } } -// FlashAutoDismiss: after a delay, clear the flash so the toast hides without user clicking X (e.g. success toasts) -Hooks.FlashAutoDismiss = { - mounted() { - const ms = this.el.dataset.autoClearMs - if (!ms) return - const delay = parseInt(ms, 10) - if (delay > 0) { - this.timer = setTimeout(() => { - const key = this.el.dataset.clearFlashKey || "success" - this.pushEvent("lv:clear-flash", {key}) - }, delay) - } - }, - - destroyed() { - if (this.timer) clearTimeout(this.timer) - } -} - // TabListKeydown hook: WCAG tab pattern — prevent default for ArrowLeft/ArrowRight so the server can handle tab switch (roving tabindex) Hooks.TabListKeydown = { mounted() { @@ -339,10 +312,7 @@ Hooks.SidebarState = { let liveSocket = new LiveSocket("/live", Socket, { longPollFallbackMs: 2500, - params: { - _csrf_token: csrfToken, - timezone: getBrowserTimezone() - }, + params: {_csrf_token: csrfToken}, hooks: Hooks }) diff --git a/config/config.exs b/config/config.exs index 7bb4f61..35e4160 100644 --- a/config/config.exs +++ b/config/config.exs @@ -46,9 +46,6 @@ config :spark, ] ] -# IANA timezone database for DateTime.shift_zone (browser timezone display) -config :elixir, :time_zone_database, Tz.TimeZoneDatabase - config :mv, ecto_repos: [Mv.Repo], generators: [timestamp_type: :utc_datetime], @@ -107,9 +104,6 @@ config :mv, :mail_from, {"Mila", "noreply@example.com"} # Join form rate limiting (Hammer). scale_ms: window in ms, limit: max submits per window per IP. config :mv, :join_rate_limit, scale_ms: 60_000, limit: 10 -# Join emails: notifier implementation (domain → web abstraction). Override in test to inject a mock. -config :mv, :join_notifier, MvWeb.JoinNotifierImpl - # Configure esbuild (the version is required) config :esbuild, version: "0.17.11", diff --git a/config/runtime.exs b/config/runtime.exs index d5ba574..1c55f64 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -32,91 +32,11 @@ get_env_or_file = fn var_name, default -> end end -# Same as get_env_or_file but raises if the value is not set or empty (after trim). -# Empty values lead to unclear runtime errors; failing at boot with a clear message is preferred. +# Same as get_env_or_file but raises if the value is not set get_env_or_file! = fn var_name, error_message -> case get_env_or_file.(var_name, nil) do - nil -> - raise error_message - - value when is_binary(value) -> - trimmed = String.trim(value) - - if trimmed == "" do - raise """ - #{error_message} - (Variable #{var_name} or #{var_name}_FILE is set but the value is empty.) - """ - else - trimmed - end - - value -> - value - end -end - -# Returns default when env_value is nil, empty after trim, or not a valid positive integer. -# Used for PORT, POOL_SIZE, SMTP_PORT to avoid ArgumentError on empty or invalid values. -parse_positive_integer = fn env_value, default -> - case env_value do - nil -> - default - - v when is_binary(v) -> - case String.trim(v) do - "" -> - default - - trimmed -> - case Integer.parse(trimmed) do - {n, _} when n > 0 -> n - _ -> default - end - end - - _ -> - default - end -end - -# Returns default when the key is missing or the value is empty (after trim). -# Use for optional string ENV vars (e.g. DATABASE_PORT) so empty string is treated as "unset". -get_env_non_empty = fn key, default -> - case System.get_env(key) do - nil -> - default - - v when is_binary(v) -> - trimmed = String.trim(v) - if trimmed == "", do: default, else: trimmed - - v -> - v - end -end - -# Returns the trimmed value when set and non-empty; otherwise raises with error_message. -# Use for required vars (DATABASE_HOST, etc.) so "set but empty" fails at boot with a clear message. -get_env_required = fn key, error_message -> - case System.get_env(key) do - nil -> - raise error_message - - v when is_binary(v) -> - trimmed = String.trim(v) - - if trimmed == "" do - raise """ - #{error_message} - (Variable #{key} is set but empty.) - """ - else - trimmed - end - - v -> - v + nil -> raise error_message + value -> value end end @@ -129,14 +49,12 @@ build_database_url = fn -> nil -> # Build URL from separate components host = - get_env_required.("DATABASE_HOST", """ - DATABASE_HOST is required when DATABASE_URL is not set. - """) + System.get_env("DATABASE_HOST") || + raise "DATABASE_HOST is required when DATABASE_URL is not set" user = - get_env_required.("DATABASE_USER", """ - DATABASE_USER is required when DATABASE_URL is not set. - """) + System.get_env("DATABASE_USER") || + raise "DATABASE_USER is required when DATABASE_URL is not set" password = get_env_or_file!.("DATABASE_PASSWORD", """ @@ -144,11 +62,10 @@ build_database_url = fn -> """) database = - get_env_required.("DATABASE_NAME", """ - DATABASE_NAME is required when DATABASE_URL is not set. - """) + System.get_env("DATABASE_NAME") || + raise "DATABASE_NAME is required when DATABASE_URL is not set" - port = get_env_non_empty.("DATABASE_PORT", "5432") + port = System.get_env("DATABASE_PORT", "5432") # URL-encode the password to handle special characters encoded_password = URI.encode_www_form(password) @@ -185,7 +102,7 @@ if config_env() == :prod do config :mv, Mv.Repo, # ssl: true, url: database_url, - pool_size: parse_positive_integer.(System.get_env("POOL_SIZE"), 10), + pool_size: String.to_integer(System.get_env("POOL_SIZE") || "10"), socket_options: maybe_ipv6 # The secret key base is used to sign/encrypt cookies and other secrets. @@ -203,14 +120,11 @@ if config_env() == :prod do # PHX_HOST or DOMAIN can be used to set the host for the application. # DOMAIN is commonly used in deployment environments (e.g., Portainer templates). host = - get_env_non_empty.("PHX_HOST", nil) || - get_env_non_empty.("DOMAIN", nil) || - raise """ - Please define the PHX_HOST or DOMAIN environment variable. - (Variable may be set but empty.) - """ + System.get_env("PHX_HOST") || + System.get_env("DOMAIN") || + raise "Please define the PHX_HOST or DOMAIN environment variable." - port = parse_positive_integer.(System.get_env("PORT"), 4000) + port = String.to_integer(System.get_env("PORT") || "4000") config :mv, :dns_cluster_query, System.get_env("DNS_CLUSTER_QUERY") @@ -312,11 +226,19 @@ if config_env() == :prod do # SMTP configuration from environment variables (overrides base adapter in prod). # When SMTP_HOST is set, configure Swoosh to use the SMTP adapter at boot time. # If SMTP is configured only via Settings (Admin UI), the mailer builds the config - # per-send at runtime using Mv.Mailer.smtp_config/0 (which uses the same Mv.Smtp.ConfigBuilder). + # per-send at runtime using Mv.Config.smtp_*() helpers. + # + # TLS/SSL options (tls_options, sockopts) are duplicated here and in Mv.Mailer.smtp_config/0 + # because boot config must be set in this file; the Mailer uses the same logic for + # Settings-only config. Keep verify behaviour in sync (see SMTP_VERIFY_PEER below). smtp_host_env = System.get_env("SMTP_HOST") if smtp_host_env && String.trim(smtp_host_env) != "" do - smtp_port_env = parse_positive_integer.(System.get_env("SMTP_PORT"), 587) + smtp_port_env = + case System.get_env("SMTP_PORT") do + nil -> 587 + v -> String.to_integer(String.trim(v)) + end smtp_password_env = case System.get_env("SMTP_PASSWORD") do @@ -342,14 +264,20 @@ if config_env() == :prod do verify_mode = if smtp_verify_peer, do: :verify_peer, else: :verify_none smtp_opts = - Mv.Smtp.ConfigBuilder.build_opts( - host: String.trim(smtp_host_env), + [ + adapter: Swoosh.Adapters.SMTP, + relay: String.trim(smtp_host_env), port: smtp_port_env, username: System.get_env("SMTP_USERNAME"), password: smtp_password_env, - ssl_mode: smtp_ssl_mode, - verify_mode: verify_mode - ) + ssl: smtp_ssl_mode == "ssl", + tls: if(smtp_ssl_mode == "tls", do: :always, else: :never), + auth: :always, + # tls_options: STARTTLS (587); sockopts: direct SSL (465). + tls_options: [verify: verify_mode], + sockopts: [verify: verify_mode] + ] + |> Enum.reject(fn {_k, v} -> is_nil(v) end) config :mv, Mv.Mailer, smtp_opts end diff --git a/config/test.exs b/config/test.exs index 7343a6a..84ccd70 100644 --- a/config/test.exs +++ b/config/test.exs @@ -58,11 +58,3 @@ config :mv, :sql_sandbox, true # Join form rate limit: low limit so tests can trigger rate limiting (e.g. 2 per minute) 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..5ff00f1 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 @@ -42,7 +42,7 @@ services: secrets: - db_password volumes: - - postgres_data_prod:/var/lib/postgresql + - postgres_data_prod:/var/lib/postgresql/data ports: - "5001:5432" restart: unless-stopped diff --git a/docker-compose.yml b/docker-compose.yml index cbd2e9e..c3473b7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,13 +4,13 @@ networks: services: db: - image: postgres:18.4-alpine + image: postgres:18.3-alpine environment: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres POSTGRES_DB: mv_dev volumes: - - postgres-data:/var/lib/postgresql + - postgres-data:/var/lib/postgresql/data ports: - "5000:5432" networks: @@ -25,7 +25,7 @@ services: rauthy: container_name: rauthy-dev - image: ghcr.io/sebadob/rauthy:0.35.2 + image: ghcr.io/sebadob/rauthy:0.34.3 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/admin-bootstrap-and-oidc-role-sync.md b/docs/admin-bootstrap-and-oidc-role-sync.md index 5413f91..5e26c85 100644 --- a/docs/admin-bootstrap-and-oidc-role-sync.md +++ b/docs/admin-bootstrap-and-oidc-role-sync.md @@ -2,7 +2,7 @@ ## Overview -- **Admin bootstrap:** In production, the Docker entrypoint runs migrate, then `Mv.Release.run_seeds/0` (skips if admin user already exists unless `FORCE_SEEDS=true`; set `RUN_DEV_SEEDS=true` to also run dev seeds), then `seed_admin/0` from ENV, then the server. Password can be changed without redeploy via `bin/mv eval "Mv.Release.seed_admin()"`. +- **Admin bootstrap:** In production, the Docker entrypoint runs migrate, then `Mv.Release.run_seeds/0` (bootstrap seeds; set `RUN_DEV_SEEDS=true` to also run dev seeds), then `seed_admin/0` from ENV, then the server. Password can be changed without redeploy via `bin/mv eval "Mv.Release.seed_admin()"`. - **OIDC role sync:** Optional mapping from OIDC groups (e.g. from Authentik profile scope) to the Admin role. Users in the configured admin group get the Admin role on registration and on each sign-in. ## Admin Bootstrap (Part A) @@ -10,14 +10,13 @@ ### Environment Variables - `RUN_DEV_SEEDS` – If set to `"true"`, `run_seeds/0` also runs dev seeds (members, groups, sample data). Otherwise only bootstrap seeds run. -- `FORCE_SEEDS` – If set to `"true"`, seeds are run even when the admin user already exists (e.g. after changing bootstrap data such as roles or custom fields). Otherwise seeds are skipped when bootstrap was already applied. - `ADMIN_EMAIL` – Email of the admin user to create/update. If unset, seed_admin/0 does nothing. - `ADMIN_PASSWORD` – Password for the admin user. If unset (and no file), no new user is created; if a user with ADMIN_EMAIL already exists (e.g. OIDC-only), their role is set to Admin (no password change). - `ADMIN_PASSWORD_FILE` – Path to a file containing the password (e.g. Docker secret). ### Release Tasks -- `Mv.Release.run_seeds/0` – If the admin user already exists (bootstrap already applied), skips unless `FORCE_SEEDS=true`; otherwise runs bootstrap seeds (fee types, custom fields, roles, settings). If `RUN_DEV_SEEDS` env is `"true"`, also runs dev seeds (members, groups, sample data). Safe to call on every start. +- `Mv.Release.run_seeds/0` – Runs bootstrap seeds (fee types, custom fields, roles, settings). If `RUN_DEV_SEEDS` env is `"true"`, also runs dev seeds (members, groups, sample data). Idempotent. - `Mv.Release.seed_admin/0` – Reads ADMIN_EMAIL and password from ADMIN_PASSWORD or ADMIN_PASSWORD_FILE. If both email and password are set: creates or updates the user with the Admin role. If only ADMIN_EMAIL is set: sets the Admin role on an existing user with that email (for OIDC-only admins); does not create a user. Idempotent. ### Entrypoint @@ -39,7 +38,6 @@ ### Sign-in page (OIDC-only mode) - `OIDC_ONLY` (or Settings → OIDC → "Only OIDC sign-in") – When set to true/1/yes and OIDC is configured, the sign-in page shows only the Single Sign-On button (password login is hidden). ENV takes precedence over Settings. -- **Redirect loop fix:** After an OIDC failure (e.g. provider down), the app redirects to `/sign-in?oidc_failed=1`. The plug `OidcOnlySignInRedirect` does not redirect that request back to OIDC, so the sign-in page is shown with the error (no endless redirect). ### Sync Logic 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/docs/development-progress-log.md b/docs/development-progress-log.md index 04c35f3..a6297ba 100644 --- a/docs/development-progress-log.md +++ b/docs/development-progress-log.md @@ -710,10 +710,6 @@ end ## Testing Strategy -### Test process environment - -`test/test_helper.exs` clears Vereinfacht and OIDC-related environment variables at startup (same rationale as not hitting real APIs when `.env` is loaded). `Mv.Config` prefers ENV over database settings; without this, OIDC sign-in redirect tests would depend on the developer shell and become flaky. Tests that need specific OIDC env values set them in `setup` and restore with `on_exit`. - ### Test Coverage Areas #### 1. Unit Tests (Domain Logic) @@ -810,7 +806,7 @@ end - **Senders migrated:** `SendNewUserConfirmationEmail`, `SendPasswordResetEmail` use layout + `Mv.Mailer.mail_from/0`. - **Cleanup:** Mix task `mix join_requests.cleanup_expired` hard-deletes JoinRequests in `pending_confirmation` with expired `confirmation_token_expires_at` (authorize?: false). For cron/Oban. - **Gettext:** New email strings in default domain; German translations in de/LC_MESSAGES/default.po; English msgstr filled for email-related strings. -- **PR review follow-ups (Join confirmation):** Join confirmation email uses `Mailer.deliver/2` with `Mailer.smtp_config/0` (same config as test mail). On delivery failure the domain returns `{:error, :email_delivery_failed}` (logged via `Logger.error`), and the JoinLive shows an error message (no success UI). Comment in `submit_join_request/2` clarifies that the raw token is hashed by `JoinRequest.Changes.SetConfirmationToken`. Cleanup task uses `Ash.bulk_destroy` and logs partial errors without halting. Layout uses assigns `app_name` and `locale` (from config/Gettext) instead of hardcoded "Mila" and `lang="de"`. Production `runtime.exs` sets `:mail_from` from ENV (`MAIL_FROM_NAME`, `MAIL_FROM_EMAIL`). Layout reference unified to `"layout.html"`; redundant `put_layout` removed from senders. +- **PR review follow-ups (Join confirmation):** Join confirmation email uses `Mailer.deliver/1` and returns `{:ok, email}` \| `{:error, reason}`; domain logs delivery errors but still returns `{:ok, request}` so the user sees success. Comment in `submit_join_request/2` clarifies that the raw token is hashed by `JoinRequest.Changes.SetConfirmationToken`. Cleanup task uses `Ash.bulk_destroy` and logs partial errors without halting. Layout uses assigns `app_name` and `locale` (from config/Gettext) instead of hardcoded "Mila" and `lang="de"`. Production `runtime.exs` sets `:mail_from` from ENV (`MAIL_FROM_NAME`, `MAIL_FROM_EMAIL`). Layout reference unified to `"layout.html"`; redundant `put_layout` removed from senders. - Tests: `join_request_test.exs`, `join_request_submit_email_test.exs`, `join_confirm_controller_test.exs` – all pass. **Subtask 3 – Admin: Join form settings (done):** diff --git a/docs/feature-roadmap.md b/docs/feature-roadmap.md index 2ec15a5..03f1cce 100644 --- a/docs/feature-roadmap.md +++ b/docs/feature-roadmap.md @@ -36,10 +36,10 @@ **Closed Issues:** - ✅ [#171](https://git.local-it.org/local-it/mitgliederverwaltung/issues/171) - OIDC handling and linking (closed 2025-11-13) -- ✅ [#146](https://git.local-it.org/local-it/mitgliederverwaltung/issues/146) - Translate "or" in the login screen — fixed via `MvWeb.AuthOverridesDE` locale-specific module (2026-03-13) -- ✅ [#144](https://git.local-it.org/local-it/mitgliederverwaltung/issues/144) - Add language switch dropdown to login screen — fixed locale selector bug with `Gettext.get_locale(MvWeb.Gettext)` (2026-03-13) -**Open Issues:** (none remaining for Authentication UI) +**Open Issues:** +- [#146](https://git.local-it.org/local-it/mitgliederverwaltung/issues/146) - Translate "or" in the login screen (Low) +- [#144](https://git.local-it.org/local-it/mitgliederverwaltung/issues/144) - Add language switch dropdown to login screen (Low) **Current State:** - ✅ **Role-based access control (RBAC)** - Implemented (2026-01-08, PR #346, closes #345) @@ -49,11 +49,6 @@ - ✅ **Page-level authorization** - LiveView page access control - ✅ **System role protection** - Critical roles cannot be deleted -**Planned: OIDC-only mode (TDD, tests first):** -- Admin Settings: When OIDC-only is enabled, disable "Allow direct registration" toggle and show hint (tests in `GlobalSettingsLiveTest`). -- Backend: Reject password sign-in and `register_with_password` when OIDC-only (tests in `AuthControllerTest`, `Accounts`). -- GET `/sign-in` redirect to OIDC when OIDC-only and OIDC configured (tests in `AuthControllerTest`). Implementation to follow after tests. - **Missing Features:** - ❌ Password reset flow - ❌ Email verification diff --git a/docs/onboarding-join-concept.md b/docs/onboarding-join-concept.md index 8e6c615..8083a7b 100644 --- a/docs/onboarding-join-concept.md +++ b/docs/onboarding-join-concept.md @@ -93,7 +93,6 @@ - **Placement:** Own section **"Onboarding / Join"** in global settings, **above** "Custom fields", **below** "Vereinsdaten" (club data). - **Join form enabled:** Checkbox (e.g. `join_form_enabled`). When set, the public `/join` page is active and the following config applies. -- **Copyable join link:** When the join form is enabled, a copyable full URL to the `/join` page is shown below the checkbox (above the field list), with a short hint so admins can share it with applicants. - **Field selection:** From **all existing** member fields (from `Mv.Constants.member_fields()`) and **custom fields**, the admin selects which fields appear on the join form. Stored as a list/set of field identifiers (no separate table); display in settings as a simple list, e.g. **badges with X to remove** (similar to the groups overview). Adding fields: e.g. dropdown or modal to pick from remaining fields. Detailed UX for this subsection is to be specified in a **separate subtask**. - **Technically required fields:** The only field that must always be required for the join flow is **email**. All other fields can be optional or marked as required per admin choice; implementation should support a "required" flag per selected join-form field. - **Other:** Which entry paths are enabled, approval workflow (who can approve) – to be detailed in Step 2 and later specs. @@ -116,7 +115,7 @@ Implementation spec for Subtask 5. #### Route and pages - **List:** **`/join_requests`** – list of join requests. Filter by status (default or primary view: status `submitted`); optional view for "all" or "approved/rejected" for audit. -- **Detail:** **`/join_requests/:id`** – single join request. **Two blocks:** (1) **Applicant data** – all form fields (typed + `form_data`) merged and shown in join-form order; (2) **Status and review** – submitted_at, status, and when decided: approved_at/rejected_at, reviewed by. Actions Approve / Reject when status is `submitted`. +- **Detail:** **`/join_requests/:id`** – single join request with all data (typed fields + `form_data`), actions Approve / Reject. #### Backend (JoinRequest) @@ -196,7 +195,7 @@ Implementation spec for Subtask 5. - **Pre-confirmation store:** **DB only.** Same JoinRequest resource; no ETS, no stateless token. Confirmation token stored as **hash** in DB; raw token only in email link. **24h** retention for `pending_confirmation`; **hard-delete** of expired records via scheduled job (e.g. Oban cron). - **Confirmation route:** **`/confirm_join/:token`** so existing `starts_with?(path, "/confirm")` covers it. - **Public path for `/join`:** **Add `/join` explicitly** to the page-permission plug’s `public_path?/1` (e.g. in `CheckPagePermission`) so unauthenticated users can reach the join page. -- **JoinRequest schema:** Status `pending_confirmation` | `submitted` | `approved` | `rejected`. Typed: **email** (required), **first_name**, **last_name** (optional). **form_data** (jsonb) + **schema_version** for remaining form fields. **confirmation_token_hash**, **confirmation_token_expires_at**; **submitted_at**, **approved_at**, **rejected_at**, **reviewed_by_user_id**, **reviewed_by_display** (denormalized reviewer email for "Geprüft von" without loading User) for audit. Idempotent confirm (unique constraint on token hash or update only when status is `pending_confirmation`). +- **JoinRequest schema:** Status `pending_confirmation` | `submitted` | `approved` | `rejected`. Typed: **email** (required), **first_name**, **last_name** (optional). **form_data** (jsonb) + **schema_version** for remaining form fields. **confirmation_token_hash**, **confirmation_token_expires_at**; **submitted_at**, **approved_at**, **rejected_at**, **reviewed_by_user_id** for audit. Idempotent confirm (unique constraint on token hash or update only when status is `pending_confirmation`). - **Approval outcome:** Admin-configurable. Default: approval creates Member only (no User). Optional "create User on approval" is **left for later**. - **Rate limiting:** Honeypot + rate limiting from the start (e.g. Hammer.Plug). - **Settings:** Own section "Onboarding / Join" in global settings; `join_form_enabled` plus field selection; display as list/badges; detailed UX in a **separate subtask**. diff --git a/docs/settings-authentication-mockup.txt b/docs/settings-authentication-mockup.txt deleted file mode 100644 index 00f64c4..0000000 --- a/docs/settings-authentication-mockup.txt +++ /dev/null @@ -1,44 +0,0 @@ -# Settings page – Authentication section (ASCII mockup) - -Structure after renaming "OIDC" to "Authentication" and adding the registration toggle. -Subsections use their own headings (h3) inside the main "Authentication" form_section. - -+------------------------------------------------------------------+ -| Settings | -| Manage global settings for the association. | -+------------------------------------------------------------------+ - -+-- Club Settings -------------------------------------------------+ -| Association Name: [________________] [Save Name] | -+------------------------------------------------------------------+ - -+-- Join Form -----------------------------------------------------+ -| ... (unchanged) | -+------------------------------------------------------------------+ - -+-- SMTP / E-Mail -------------------------------------------------+ -| ... | -+------------------------------------------------------------------+ - -+-- Accounting-Software (Vereinfacht) Integration -----------------+ -| ... | -+------------------------------------------------------------------+ - -+-- Authentication ------------------------------------------------+ <-- main section (renamed from "OIDC (Single Sign-On)") -| | -| Direct registration | <-- subsection heading (h3) -| [x] Allow direct registration (/register) | -| If disabled, users cannot sign up via /register; sign-in | -| and the join form remain available. | -| | -| OIDC (Single Sign-On) | <-- subsection heading (h3) -| (Some values are set via environment variables...) | -| Client ID: [________________] | -| Base URL: [________________] | -| Redirect URI: [________________] | -| Client Secret: [________________] (set) | -| Admin group name: [________________] | -| Groups claim: [________________] | -| [ ] Only OIDC sign-in (hide password login) | -| [Save OIDC Settings] | -+------------------------------------------------------------------+ diff --git a/docs/smtp-configuration-concept.md b/docs/smtp-configuration-concept.md index b19f6e4..30fd7de 100644 --- a/docs/smtp-configuration-concept.md +++ b/docs/smtp-configuration-concept.md @@ -25,11 +25,7 @@ Enable configurable SMTP for sending transactional emails (join confirmation, us | ENV | 1 | Production, Docker, 12-factor | | Settings | 2 | Admin UI, dev without ENV | -When `SMTP_HOST` is set, SMTP runs in **ENV-only mode**: -- all SMTP fields in Settings are read-only, -- saving SMTP settings in the UI is disabled, -- and the UI shows a warning block if required SMTP ENV values are missing. -- the UI displays the effective ENV-driven SMTP values in disabled fields so admins can verify what is active. +When an ENV variable is set, the corresponding Settings field is read-only in the UI (with hint "Set by environment"). --- @@ -46,12 +42,8 @@ When `SMTP_HOST` is set, SMTP runs in **ENV-only mode**: | Sender name | `MAIL_FROM_NAME` | `smtp_from_name` | Display name in "From" header (default: Mila)| | Sender email | `MAIL_FROM_EMAIL` | `smtp_from_email` | Address in "From" header; must match SMTP user on most servers | -**Boot-time ENV handling:** In `config/runtime.exs`, if `SMTP_PORT` is set but empty or invalid, it is treated as unset and default 587 is used. This avoids startup crashes (e.g. `ArgumentError` from `String.to_integer("")`) when variables are misconfigured in deployment. - **Important:** On most SMTP servers (e.g. Postfix with strict relay policies) the sender email (`smtp_from_email`) must be the same address as `smtp_username` or an alias that is owned by that account. -**Settings UI:** The form uses three rows on wide viewports: host, port, TLS/SSL | username, password | sender email, sender name. Content width is limited by the global settings wrapper (see `DESIGN_GUIDELINES.md` §6.4). - --- ## 5. Password from File @@ -67,14 +59,6 @@ Support **SMTP_PASSWORD_FILE** (path to file containing the password), same patt - Show a warning in the Settings UI. - Delivery attempts silently fall back to the Local adapter (no crash). -### 6.1 Behaviour in ENV-only mode (`SMTP_HOST` set) - -- The SMTP source of truth is environment variables only. -- The UI does not allow editing SMTP fields in this mode. -- The Settings page shows a warning block when required values are missing: - - `SMTP_USERNAME` - - `SMTP_PASSWORD` or `SMTP_PASSWORD_FILE` - --- ## 7. Test Email (Settings UI) @@ -98,19 +82,13 @@ Provided by `Mv.Config.mail_from_name/0` and `Mv.Config.mail_from_email/0`. --- -## 9. Join Confirmation Email - -`MvWeb.Emails.JoinConfirmationEmail` uses the same SMTP configuration as the test email: `Mailer.deliver(email, Mailer.smtp_config())`. This ensures Settings-based SMTP is used when not configured via ENV at boot. On delivery failure the domain returns `{:error, :email_delivery_failed}` (and logs via `Logger.error`); the JoinLive shows an error message and no success UI. - ---- - -## 10. AshAuthentication Senders +## 9. AshAuthentication Senders Both `SendPasswordResetEmail` and `SendNewUserConfirmationEmail` use `Mv.Mailer.deliver/1` (not `deliver!/1`). Delivery failures are logged (`Logger.error`) and not re-raised, so they never crash the caller process. AshAuthentication ignores the return value of `send/3`. --- -## 11. TLS / SSL in OTP 27 +## 10. TLS / SSL in OTP 27 OTP 26+ enforces `verify_peer` by default, which fails for self-signed or internal SMTP server certificates. @@ -119,13 +97,11 @@ By default, TLS certificate verification is relaxed (`verify_none`) so self-sign - **ENV (prod):** Set `SMTP_VERIFY_PEER=true` (or `1`/`yes`) when configuring SMTP via environment variables in `config/runtime.exs`. This sets `config :mv, :smtp_verify_peer` and is used for both boot-time and per-send config. - **Default:** `false` (verify_none) for backward compatibility and internal/self-signed certs. -Verify mode is set in `tls_options` for port 587 (STARTTLS). For port 465 (implicit SSL), the initial connection is `ssl:connect`, so we also pass `sockopts: [verify: verify_mode]` so the SSL handshake uses the same mode. For 587 we must not pass `verify` in sockopts—gen_tcp is used first and rejects it (ArgumentError). The logic lives in `Mv.Smtp.ConfigBuilder.build_opts/1` (single source of truth), used by `config/runtime.exs` (boot) and `Mv.Mailer.smtp_config/0` (Settings-only). - -**Tests:** `Mv.Smtp.ConfigBuilderTest` asserts sockopts/TLS shape. `Mv.Mailer.smtp_config/0` returns `[]` when the mailer adapter is `Swoosh.Adapters.Test`; `test/mv/mailer_smtp_config_test.exs` asserts that guard and, with the adapter temporarily set to `Swoosh.Adapters.Local`, wiring from ENV. Those mailer tests use `Mv.DataCase` so Settings fallbacks in `Mv.Config` (e.g. SMTP username/password when ENV is unset) stay under the SQL sandbox. +Both `tls_options` (STARTTLS, port 587) and `sockopts` (direct SSL, port 465) use the same verify mode. The logic is duplicated in `config/runtime.exs` (boot) and `Mv.Mailer.smtp_config/0` (Settings-only); keep in sync. --- -## 12. Summary Checklist +## 11. Summary Checklist - [x] ENV: `SMTP_HOST`, `SMTP_PORT`, `SMTP_USERNAME`, `SMTP_PASSWORD`, `SMTP_PASSWORD_FILE`, `SMTP_SSL`. - [x] ENV: `MAIL_FROM_NAME`, `MAIL_FROM_EMAIL` for sender identity. @@ -133,17 +109,16 @@ Verify mode is set in `tls_options` for port 587 (STARTTLS). For port 465 (impli - [x] Password from file: `SMTP_PASSWORD_FILE` supported in `runtime.exs`. - [x] Mailer: Swoosh SMTP adapter configured from merged ENV + Settings when SMTP is configured. - [x] Per-request SMTP config via `Mv.Mailer.smtp_config/0` for Settings-only scenarios. -- [x] TLS certificate validation relaxed for OTP 27 (tls_options for 587; sockopts with verify only for 465). +- [x] TLS certificate validation relaxed for OTP 27 (tls_options + sockopts). - [x] Prod warning: clear message in Settings when SMTP is not configured. - [x] Test email: form with recipient field, translatable content, classified success/error messages. -- [x] Join confirmation email: uses `Mailer.smtp_config/0` (same as test mail); on failure returns `{:error, :email_delivery_failed}`, error shown in JoinLive, logged for admin. - [x] AshAuthentication senders: graceful error handling (no crash on delivery failure). - [x] Gettext for all new UI strings, translated to German. - [x] Docs and code guidelines updated. --- -## 13. Follow-up / Future Work +## 12. Follow-up / Future Work - **SMTP password at-rest encryption:** The `smtp_password` attribute is currently stored in plaintext in the `settings` table. It is excluded from default reads (same pattern as `oidc_client_secret`); both are read only via explicit select when needed. For production systems at-rest encryption (e.g. with [Cloak](https://hexdocs.pm/cloak)) should be considered and tracked as a follow-up issue. - **Error classification:** SMTP error categorization currently uses substring matching on server messages (e.g. "535", "authentication"). A more robust approach would be to pattern-match on `gen_smtp` error tuples first where possible, and fall back to string analysis only when needed. Server wording varies; consider extending patterns as new providers are used. diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index 0127796..6b9cd1e 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -362,12 +362,6 @@ defmodule Mv.Accounts.User do # Authorization Policies # Order matters: Most specific policies first, then general permission check policies do - # When OIDC-only is active, password sign-in is forbidden (SSO only). - policy action(:sign_in_with_password) do - forbid_if Mv.Authorization.Checks.OidcOnlyActive - authorize_if always() - end - # AshAuthentication bypass (registration/login without actor) bypass AshAuthentication.Checks.AshAuthenticationInteraction do description "Allow AshAuthentication internal operations (registration, login)" @@ -411,14 +405,6 @@ defmodule Mv.Accounts.User do where: [action_is([:register_with_password, :admin_set_password])], message: "must have length of at least 8" - # Block direct registration when disabled in global settings - validate {Mv.Accounts.User.Validations.RegistrationEnabled, []}, - where: [action_is(:register_with_password)] - - # Block password registration when OIDC-only mode is active - validate {Mv.Accounts.User.Validations.OidcOnlyBlocksPasswordRegistration, []}, - where: [action_is(:register_with_password)] - # Email uniqueness check for all actions that change the email attribute # Validates that user email is not already used by another (unlinked) member validate Mv.Accounts.User.Validations.EmailNotUsedByOtherMember diff --git a/lib/accounts/user/validations/oidc_only_blocks_password_registration.ex b/lib/accounts/user/validations/oidc_only_blocks_password_registration.ex deleted file mode 100644 index e4d9a35..0000000 --- a/lib/accounts/user/validations/oidc_only_blocks_password_registration.ex +++ /dev/null @@ -1,27 +0,0 @@ -defmodule Mv.Accounts.User.Validations.OidcOnlyBlocksPasswordRegistration do - @moduledoc """ - Validation that blocks direct registration (register_with_password) when - OIDC-only mode is active. In OIDC-only mode, sign-in and registration are - only allowed via OIDC (SSO). - """ - use Ash.Resource.Validation - - @impl true - def init(opts), do: {:ok, opts} - - @impl true - def validate(_changeset, _opts, _context) do - if Mv.Config.oidc_only?() do - {:error, - field: :base, - message: - Gettext.dgettext( - MvWeb.Gettext, - "default", - "Registration with password is disabled when only OIDC sign-in is active." - )} - else - :ok - end - end -end diff --git a/lib/accounts/user/validations/registration_enabled.ex b/lib/accounts/user/validations/registration_enabled.ex deleted file mode 100644 index f2342b7..0000000 --- a/lib/accounts/user/validations/registration_enabled.ex +++ /dev/null @@ -1,31 +0,0 @@ -defmodule Mv.Accounts.User.Validations.RegistrationEnabled do - @moduledoc """ - Validation that blocks direct registration (register_with_password) when - registration is disabled in global settings. Used so that even direct API/form - submissions cannot register when the setting is off. - """ - use Ash.Resource.Validation - - alias Mv.Membership - - @impl true - def init(opts), do: {:ok, opts} - - @impl true - def validate(_changeset, _opts, _context) do - case Membership.get_settings() do - {:ok, %{registration_enabled: true}} -> - :ok - - _ -> - {:error, - field: :base, - message: - Gettext.dgettext( - MvWeb.Gettext, - "default", - "Registration is disabled. Please use the join form or contact an administrator." - )} - end - end -end 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_notifier.ex b/lib/membership/join_notifier.ex deleted file mode 100644 index daec4c1..0000000 --- a/lib/membership/join_notifier.ex +++ /dev/null @@ -1,13 +0,0 @@ -defmodule Mv.Membership.JoinNotifier do - @moduledoc """ - Behaviour for sending join-related emails (confirmation, already member, already pending). - - The domain calls this module instead of MvWeb.Emails directly, so the domain layer - does not depend on the web layer. The default implementation is set in config - (`config :mv, :join_notifier, MvWeb.JoinNotifierImpl`). Tests can override with a mock. - """ - @callback send_confirmation(email :: String.t(), token :: String.t(), opts :: keyword()) :: - {:ok, term()} | {:error, term()} - @callback send_already_member(email :: String.t()) :: {:ok, term()} | {:error, term()} - @callback send_already_pending(email :: String.t()) :: {:ok, term()} | {:error, term()} -end diff --git a/lib/membership/join_request.ex b/lib/membership/join_request.ex index 94907e2..05a9e8d 100644 --- a/lib/membership/join_request.ex +++ b/lib/membership/join_request.ex @@ -77,17 +77,6 @@ defmodule Mv.Membership.JoinRequest do change Mv.Membership.JoinRequest.Changes.RejectRequest end - - # Internal: resend confirmation (new token) when user submits form again with same email. - # Called from domain with authorize?: false; not exposed to public. - update :regenerate_confirmation_token do - description "Set new confirmation token and expiry (resend flow)" - require_atomic? false - - argument :confirmation_token, :string, allow_nil?: false - - change Mv.Membership.JoinRequest.Changes.RegenerateConfirmationToken - end end policies do @@ -186,11 +175,6 @@ defmodule Mv.Membership.JoinRequest do attribute :approved_at, :utc_datetime_usec attribute :rejected_at, :utc_datetime_usec attribute :reviewed_by_user_id, :uuid - - attribute :reviewed_by_display, :string do - description "Denormalized reviewer display (e.g. email) for UI without loading User" - end - attribute :source, :string create_timestamp :inserted_at diff --git a/lib/membership/join_request/changes/approve_request.ex b/lib/membership/join_request/changes/approve_request.ex index b86ca5d..24716f6 100644 --- a/lib/membership/join_request/changes/approve_request.ex +++ b/lib/membership/join_request/changes/approve_request.ex @@ -16,13 +16,11 @@ defmodule Mv.Membership.JoinRequest.Changes.ApproveRequest do if current_status == :submitted do reviewed_by_id = Helpers.actor_id(context.actor) - reviewed_by_display = Helpers.actor_email(context.actor) changeset |> Ash.Changeset.force_change_attribute(:status, :approved) |> Ash.Changeset.force_change_attribute(:approved_at, DateTime.utc_now()) |> Ash.Changeset.force_change_attribute(:reviewed_by_user_id, reviewed_by_id) - |> Ash.Changeset.force_change_attribute(:reviewed_by_display, reviewed_by_display) else Ash.Changeset.add_error(changeset, field: :status, 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/join_request/changes/helpers.ex b/lib/membership/join_request/changes/helpers.ex index 9bb0697..ee09b75 100644 --- a/lib/membership/join_request/changes/helpers.ex +++ b/lib/membership/join_request/changes/helpers.ex @@ -16,24 +16,4 @@ defmodule Mv.Membership.JoinRequest.Changes.Helpers do end def actor_id(_), do: nil - - @doc """ - Extracts the actor's email for display (e.g. reviewed_by_display). - - Supports both atom and string keys for compatibility with different actor representations. - """ - @spec actor_email(term()) :: String.t() | nil - def actor_email(nil), do: nil - - def actor_email(actor) when is_map(actor) do - raw = Map.get(actor, :email) || Map.get(actor, "email") - if is_nil(raw), do: nil, else: actor_email_string(raw) - end - - def actor_email(_), do: nil - - defp actor_email_string(raw) do - s = raw |> to_string() |> String.trim() - if s == "", do: nil, else: s - end end diff --git a/lib/membership/join_request/changes/regenerate_confirmation_token.ex b/lib/membership/join_request/changes/regenerate_confirmation_token.ex deleted file mode 100644 index c8055d2..0000000 --- a/lib/membership/join_request/changes/regenerate_confirmation_token.ex +++ /dev/null @@ -1,33 +0,0 @@ -defmodule Mv.Membership.JoinRequest.Changes.RegenerateConfirmationToken do - @moduledoc """ - Sets a new confirmation token hash and expiry on an existing join request (resend flow). - - Used when the user submits the join form again with the same email while a request - is still pending_confirmation. Internal use only (domain calls with authorize?: false). - """ - use Ash.Resource.Change - - alias Mv.Membership.JoinRequest - - @confirmation_validity_hours 24 - - @spec change(Ash.Changeset.t(), keyword(), Ash.Resource.Change.context()) :: Ash.Changeset.t() - def change(changeset, _opts, _context) do - token = Ash.Changeset.get_argument(changeset, :confirmation_token) - - if is_binary(token) and token != "" do - now = DateTime.utc_now() - expires_at = DateTime.add(now, @confirmation_validity_hours, :hour) - - changeset - |> Ash.Changeset.force_change_attribute( - :confirmation_token_hash, - JoinRequest.hash_confirmation_token(token) - ) - |> Ash.Changeset.force_change_attribute(:confirmation_token_expires_at, expires_at) - |> Ash.Changeset.force_change_attribute(:confirmation_sent_at, now) - else - changeset - end - end -end diff --git a/lib/membership/join_request/changes/reject_request.ex b/lib/membership/join_request/changes/reject_request.ex index 1b9fe1a..2c33a77 100644 --- a/lib/membership/join_request/changes/reject_request.ex +++ b/lib/membership/join_request/changes/reject_request.ex @@ -15,13 +15,11 @@ defmodule Mv.Membership.JoinRequest.Changes.RejectRequest do if current_status == :submitted do reviewed_by_id = Helpers.actor_id(context.actor) - reviewed_by_display = Helpers.actor_email(context.actor) changeset |> Ash.Changeset.force_change_attribute(:status, :rejected) |> Ash.Changeset.force_change_attribute(:rejected_at, DateTime.utc_now()) |> Ash.Changeset.force_change_attribute(:reviewed_by_user_id, reviewed_by_id) - |> Ash.Changeset.force_change_attribute(:reviewed_by_display, reviewed_by_display) else Ash.Changeset.add_error(changeset, field: :status, 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..2f18f90 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -29,10 +29,8 @@ defmodule Mv.Membership do require Ash.Query import Ash.Expr alias Ash.Error.Query.NotFound, as: NotFoundError - alias Mv.Helpers.SystemActor alias Mv.Membership.JoinRequest - alias Mv.Membership.Member - alias Mv.Membership.SettingsCache + alias MvWeb.Emails.JoinConfirmationEmail require Logger admin do @@ -116,16 +114,10 @@ defmodule Mv.Membership do """ def get_settings do - case Process.whereis(SettingsCache) do - nil -> get_settings_uncached() - _pid -> SettingsCache.get() - end - end - - @doc false - def get_settings_uncached do + # Try to get the first (and only) settings record case Ash.read_one(Mv.Membership.Setting, domain: __MODULE__) do {:ok, nil} -> + # No settings exist - create as fallback (should normally be created via seed script) default_club_name = System.get_env("ASSOCIATION_NAME") || "Club Name" Mv.Membership.Setting @@ -166,16 +158,9 @@ defmodule Mv.Membership do """ def update_settings(settings, attrs) do - case settings - |> Ash.Changeset.for_update(:update, attrs) - |> Ash.update(domain: __MODULE__) do - {:ok, _updated} = result -> - SettingsCache.invalidate() - result - - error -> - error - end + settings + |> Ash.Changeset.for_update(:update, attrs) + |> Ash.update(domain: __MODULE__) end @doc """ @@ -239,18 +224,11 @@ defmodule Mv.Membership do """ def update_member_field_visibility(settings, visibility_config) do - case settings - |> Ash.Changeset.for_update(:update_member_field_visibility, %{ - member_field_visibility: visibility_config - }) - |> Ash.update(domain: __MODULE__) do - {:ok, _} = result -> - SettingsCache.invalidate() - result - - error -> - error - end + settings + |> Ash.Changeset.for_update(:update_member_field_visibility, %{ + member_field_visibility: visibility_config + }) + |> Ash.update(domain: __MODULE__) end @doc """ @@ -283,19 +261,12 @@ defmodule Mv.Membership do field: field, show_in_overview: show_in_overview ) do - case settings - |> Ash.Changeset.new() - |> Ash.Changeset.set_argument(:field, field) - |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) - |> Ash.Changeset.for_update(:update_single_member_field_visibility, %{}) - |> Ash.update(domain: __MODULE__) do - {:ok, _} = result -> - SettingsCache.invalidate() - result - - error -> - error - end + settings + |> Ash.Changeset.new() + |> Ash.Changeset.set_argument(:field, field) + |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) + |> Ash.Changeset.for_update(:update_single_member_field_visibility, %{}) + |> Ash.update(domain: __MODULE__) end @doc """ @@ -329,20 +300,13 @@ defmodule Mv.Membership do show_in_overview: show_in_overview, required: required ) do - case settings - |> Ash.Changeset.new() - |> Ash.Changeset.set_argument(:field, field) - |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) - |> Ash.Changeset.set_argument(:required, required) - |> Ash.Changeset.for_update(:update_single_member_field, %{}) - |> Ash.update(domain: __MODULE__) do - {:ok, _} = result -> - SettingsCache.invalidate() - result - - error -> - error - end + settings + |> Ash.Changeset.new() + |> Ash.Changeset.set_argument(:field, field) + |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) + |> Ash.Changeset.set_argument(:required, required) + |> Ash.Changeset.for_update(:update_single_member_field, %{}) + |> Ash.update(domain: __MODULE__) end @doc """ @@ -400,131 +364,15 @@ defmodule Mv.Membership do - `:actor` - Must be nil for public submit (policy allows only unauthenticated). ## Returns - - `{:ok, request}` - Created JoinRequest in status pending_confirmation, email sent - - `{:ok, :notified_already_member}` - Email already a member; notice sent by email only (no request created) - - `{:ok, :notified_already_pending}` - Email already has pending/submitted request; notice or resend sent by email only - - `{:error, :email_delivery_failed}` - Request created but confirmation email could not be sent (logged) + - `{:ok, request}` - Created JoinRequest in status pending_confirmation - `{:error, error}` - Validation or authorization error """ def submit_join_request(attrs, opts \\ []) do actor = Keyword.get(opts, :actor) - email = normalize_submit_email(attrs) - - pending = - if email != nil and email != "", do: pending_join_request_with_email(email), else: nil - - cond do - email != nil and email != "" and member_exists_with_email?(email) -> - send_already_member_and_return(email) - - pending != nil -> - handle_already_pending(email, pending) - - true -> - do_create_join_request(attrs, actor) - end - end - - defp normalize_submit_email(attrs) do - raw = attrs["email"] || attrs[:email] - if is_binary(raw), do: String.trim(raw), else: nil - end - - defp member_exists_with_email?(email) when is_binary(email) do - system_actor = SystemActor.get_system_actor() - opts = [actor: system_actor, domain: __MODULE__] - - case Ash.get(Member, %{email: email}, opts) do - {:ok, _member} -> true - _ -> false - end - end - - defp member_exists_with_email?(_), do: false - - defp pending_join_request_with_email(email) when is_binary(email) do - system_actor = SystemActor.get_system_actor() - - query = - JoinRequest - |> Ash.Query.filter(expr(email == ^email and status in [:pending_confirmation, :submitted])) - |> Ash.Query.sort(inserted_at: :desc) - |> Ash.Query.limit(1) - - case Ash.read_one(query, actor: system_actor, domain: __MODULE__) do - {:ok, request} -> request - _ -> nil - end - end - - defp pending_join_request_with_email(_), do: nil - - defp join_notifier do - Application.get_env(:mv, :join_notifier, MvWeb.JoinNotifierImpl) - end - - defp send_already_member_and_return(email) do - case join_notifier().send_already_member(email) do - {:ok, _} -> - :ok - - {:error, reason} -> - Logger.error("Join already-member email failed for #{email}: #{inspect(reason)}") - end - - # Delay is applied by the caller (e.g. JoinLive) to avoid blocking the process. - {:ok, :notified_already_member} - end - - defp handle_already_pending(email, existing) do - if existing.status == :pending_confirmation do - resend_confirmation_to_pending(email, existing) - else - send_already_pending_and_return(email) - end - end - - defp resend_confirmation_to_pending(email, request) do - new_token = generate_confirmation_token() - - case request - |> Ash.Changeset.for_update(:regenerate_confirmation_token, %{ - confirmation_token: new_token - }) - |> Ash.update(domain: __MODULE__, authorize?: false) do - {:ok, _updated} -> - case join_notifier().send_confirmation(email, new_token, resend: true) do - {:ok, _} -> - :ok - - {:error, reason} -> - Logger.error("Join resend confirmation email failed for #{email}: #{inspect(reason)}") - end - - # Delay is applied by the caller (e.g. JoinLive) to avoid blocking the process. - {:ok, :notified_already_pending} - - {:error, _} -> - # Fallback: do not create duplicate; send generic pending email - send_already_pending_and_return(email) - end - end - - defp send_already_pending_and_return(email) do - case join_notifier().send_already_pending(email) do - {:ok, _} -> - :ok - - {:error, reason} -> - Logger.error("Join already-pending email failed for #{email}: #{inspect(reason)}") - end - - # Delay is applied by the caller (e.g. JoinLive) to avoid blocking the process. - {:ok, :notified_already_pending} - end - - defp do_create_join_request(attrs, actor) do token = Map.get(attrs, :confirmation_token) || generate_confirmation_token() + + # Raw token is passed to the submit action; JoinRequest.Changes.SetConfirmationToken + # hashes it before persist. Only the hash is stored; the raw token is sent in the email link. attrs_with_token = Map.put(attrs, :confirmation_token, token) case Ash.create(JoinRequest, attrs_with_token, @@ -533,9 +381,8 @@ defmodule Mv.Membership do domain: __MODULE__ ) do {:ok, request} -> - case join_notifier().send_confirmation(request.email, token, []) do + case JoinConfirmationEmail.send(request.email, token) do {:ok, _email} -> - # Delay is applied by the caller (e.g. JoinLive) to avoid blocking the process. {:ok, request} {:error, reason} -> @@ -543,7 +390,8 @@ defmodule Mv.Membership do "Join confirmation email failed for #{request.email}: #{inspect(reason)}" ) - {:error, :email_delivery_failed} + # Request was created; return success so the user sees the confirmation message + {:ok, request} end error -> @@ -836,10 +684,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/setting.ex b/lib/membership/setting.ex index 83c5c8b..ce63589 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -15,7 +15,6 @@ defmodule Mv.Membership.Setting do (e.g., `%{"first_name" => true, "last_name" => true}`). Email is always required; other fields default to optional. - `include_joining_cycle` - Whether to include the joining cycle in membership fee generation (default: true) - `default_membership_fee_type_id` - Default membership fee type for new members (optional) - - `registration_enabled` - Whether direct registration via /register is allowed (default: true) - `join_form_enabled` - Whether the public /join page is active (default: false) - `join_form_field_ids` - Ordered list of field IDs shown on the join form. Each entry is either a member field name string (e.g. "email") or a custom field UUID. Email is always @@ -130,7 +129,6 @@ defmodule Mv.Membership.Setting do :smtp_ssl, :smtp_from_name, :smtp_from_email, - :registration_enabled, :join_form_enabled, :join_form_field_ids, :join_form_field_required @@ -167,7 +165,6 @@ defmodule Mv.Membership.Setting do :smtp_ssl, :smtp_from_name, :smtp_from_email, - :registration_enabled, :join_form_enabled, :join_form_field_ids, :join_form_field_required @@ -517,15 +514,6 @@ defmodule Mv.Membership.Setting do description "Email address for the transactional email sender. Must be owned by the SMTP user. Overrides MAIL_FROM_EMAIL env." end - # Authentication: direct registration toggle - attribute :registration_enabled, :boolean do - allow_nil? false - default true - public? true - - description "When true, users can register via /register; when false, only sign-in and join form remain available." - end - # Join form (Beitrittsformular) settings attribute :join_form_enabled, :boolean do allow_nil? false diff --git a/lib/membership/settings_cache.ex b/lib/membership/settings_cache.ex deleted file mode 100644 index d8581d6..0000000 --- a/lib/membership/settings_cache.ex +++ /dev/null @@ -1,85 +0,0 @@ -defmodule Mv.Membership.SettingsCache do - @moduledoc """ - Process-based cache for global settings to avoid repeated DB reads on hot paths - (e.g. RegistrationEnabled validation, Layouts.public_page, Plugs). - - Uses a short TTL (default 60 seconds). Cache is invalidated on every settings - update so that changes take effect quickly. If no settings process exists - (e.g. in tests), get/1 falls back to direct read. - """ - use GenServer - - @default_ttl_seconds 60 - - def start_link(opts \\ []) do - GenServer.start_link(__MODULE__, opts, name: __MODULE__) - end - - @doc """ - Returns cached settings or fetches and caches them. Uses TTL; invalidate on update. - """ - def get do - case Process.whereis(__MODULE__) do - nil -> - # No cache process (e.g. test) – read directly - do_fetch() - - _pid -> - GenServer.call(__MODULE__, :get, 10_000) - end - end - - @doc """ - Invalidates the cache so the next get/0 will refetch from the database. - Call after update_settings and any other path that mutates settings. - """ - def invalidate do - case Process.whereis(__MODULE__) do - nil -> :ok - _pid -> GenServer.cast(__MODULE__, :invalidate) - end - end - - @impl true - def init(opts) do - ttl = Keyword.get(opts, :ttl_seconds, @default_ttl_seconds) - state = %{ttl_seconds: ttl, cached: nil, expires_at: nil} - {:ok, state} - end - - @impl true - def handle_call(:get, _from, state) do - now = System.monotonic_time(:second) - expired? = state.expires_at == nil or state.expires_at <= now - - {result, new_state} = - if expired? do - fetch_and_cache(now, state) - else - {{:ok, state.cached}, state} - end - - {:reply, result, new_state} - end - - defp fetch_and_cache(now, state) do - case do_fetch() do - {:ok, settings} = ok -> - expires = now + state.ttl_seconds - {ok, %{state | cached: settings, expires_at: expires}} - - err -> - result = if state.cached, do: {:ok, state.cached}, else: err - {result, state} - end - end - - @impl true - def handle_cast(:invalidate, state) do - {:noreply, %{state | cached: nil, expires_at: nil}} - end - - defp do_fetch do - Mv.Membership.get_settings_uncached() - end -end 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/application.ex b/lib/mv/application.ex index 1b6014e..6b4a10b 100644 --- a/lib/mv/application.ex +++ b/lib/mv/application.ex @@ -6,7 +6,6 @@ defmodule Mv.Application do use Application alias Mv.Helpers.SystemActor - alias Mv.Membership.SettingsCache alias Mv.Repo alias Mv.Vereinfacht.SyncFlash alias MvWeb.Endpoint @@ -17,28 +16,20 @@ defmodule Mv.Application do def start(_type, _args) do SyncFlash.create_table!() - # SettingsCache not started in test so get_settings runs in the test process (Ecto Sandbox). - cache_children = - if Application.get_env(:mv, :environment) == :test, do: [], else: [SettingsCache] - - children = - [ - Telemetry, - Repo - ] ++ - cache_children ++ - [ - {JoinRateLimit, [clean_period: :timer.minutes(1)]}, - {Task.Supervisor, name: Mv.TaskSupervisor}, - {DNSCluster, query: Application.get_env(:mv, :dns_cluster_query) || :ignore}, - {Phoenix.PubSub, name: Mv.PubSub}, - {AshAuthentication.Supervisor, otp_app: :my}, - SystemActor, - # Start a worker by calling: Mv.Worker.start_link(arg) - # {Mv.Worker, arg}, - # Start to serve requests, typically the last entry - Endpoint - ] + children = [ + Telemetry, + Repo, + {JoinRateLimit, [clean_period: :timer.minutes(1)]}, + {Task.Supervisor, name: Mv.TaskSupervisor}, + {DNSCluster, query: Application.get_env(:mv, :dns_cluster_query) || :ignore}, + {Phoenix.PubSub, name: Mv.PubSub}, + {AshAuthentication.Supervisor, otp_app: :my}, + SystemActor, + # Start a worker by calling: Mv.Worker.start_link(arg) + # {Mv.Worker, arg}, + # Start to serve requests, typically the last entry + Endpoint + ] # See https://hexdocs.pm/elixir/Supervisor.html # for other strategies and supported options diff --git a/lib/mv/authorization/checks/oidc_only_active.ex b/lib/mv/authorization/checks/oidc_only_active.ex deleted file mode 100644 index 8d56ca1..0000000 --- a/lib/mv/authorization/checks/oidc_only_active.ex +++ /dev/null @@ -1,16 +0,0 @@ -defmodule Mv.Authorization.Checks.OidcOnlyActive do - @moduledoc """ - Policy check: true when OIDC-only mode is active (Config.oidc_only?()). - - Used to forbid password sign-in when only OIDC (SSO) sign-in is allowed. - """ - use Ash.Policy.SimpleCheck - - alias Mv.Config - - @impl true - def describe(_opts), do: "OIDC-only mode is active" - - @impl true - def match?(_actor, _context, _opts), do: Config.oidc_only?() -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/authorization/role.ex b/lib/mv/authorization/role.ex index 1e03151..77e0507 100644 --- a/lib/mv/authorization/role.ex +++ b/lib/mv/authorization/role.ex @@ -43,6 +43,11 @@ defmodule Mv.Authorization.Role do postgres do table "roles" repo Mv.Repo + + references do + # Prevent deletion of roles that are assigned to users + reference :users, on_delete: :restrict + end end code_interface do diff --git a/lib/mv/config.ex b/lib/mv/config.ex index 750a7db..3494937 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" @@ -463,77 +470,56 @@ defmodule Mv.Config do # --------------------------------------------------------------------------- @doc """ - Returns SMTP host. - - Policy: - - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_HOST` - - Settings mode: read from Settings only + Returns SMTP host. ENV `SMTP_HOST` overrides Settings. """ @spec smtp_host() :: String.t() | nil def smtp_host do - if smtp_env_mode?() do - System.get_env("SMTP_HOST") |> trim_nil() - else - get_from_settings(:smtp_host) - end + smtp_env_or_setting("SMTP_HOST", :smtp_host) end @doc """ - Returns SMTP port as integer. - - Policy: - - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_PORT` - - Settings mode: read from Settings only + Returns SMTP port as integer. ENV `SMTP_PORT` (parsed) overrides Settings. + Returns nil when neither ENV nor Settings provide a valid port. """ - @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")) - else - get_from_settings_integer(:smtp_port) + case System.get_env("SMTP_PORT") do + nil -> + get_from_settings_integer(:smtp_port) + + value when is_binary(value) -> + case Integer.parse(String.trim(value)) do + {port, _} when port > 0 -> port + _ -> nil + end end end @doc """ - Returns SMTP username. - - Policy: - - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_USERNAME` - - Settings mode: read from Settings only + Returns SMTP username. ENV `SMTP_USERNAME` overrides Settings. """ @spec smtp_username() :: String.t() | nil def smtp_username do - if smtp_env_mode?() do - System.get_env("SMTP_USERNAME") |> trim_nil() - else - get_from_settings(:smtp_username) - end + smtp_env_or_setting("SMTP_USERNAME", :smtp_username) end @doc """ Returns SMTP password. - Policy: - - ENV-only mode (`SMTP_HOST` set): `SMTP_PASSWORD` > `SMTP_PASSWORD_FILE` - - Settings mode: read from Settings only - + Priority: `SMTP_PASSWORD` ENV > `SMTP_PASSWORD_FILE` (file contents) > Settings. Strips trailing whitespace/newlines from file contents. """ @spec smtp_password() :: String.t() | nil def smtp_password do - if smtp_env_mode?() do - case System.get_env("SMTP_PASSWORD") do - nil -> smtp_password_from_file() - value -> trim_nil(value) - end - else - get_smtp_password_from_settings() + case System.get_env("SMTP_PASSWORD") do + nil -> smtp_password_from_file_or_settings() + value -> trim_nil(value) end end - defp smtp_password_from_file do + defp smtp_password_from_file_or_settings do case System.get_env("SMTP_PASSWORD_FILE") do - nil -> nil + nil -> get_smtp_password_from_settings() path -> read_smtp_password_file(path) end end @@ -547,18 +533,11 @@ defmodule Mv.Config do @doc """ Returns SMTP TLS/SSL mode string (e.g. 'tls', 'ssl', 'none'). - - Policy: - - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_SSL` - - Settings mode: read from Settings only + ENV `SMTP_SSL` overrides Settings. """ @spec smtp_ssl() :: String.t() | nil def smtp_ssl do - if smtp_env_mode?() do - System.get_env("SMTP_SSL") |> trim_nil() - else - get_from_settings(:smtp_ssl) - end + smtp_env_or_setting("SMTP_SSL", :smtp_ssl) end @doc """ @@ -570,32 +549,12 @@ defmodule Mv.Config do end @doc """ - Returns true when SMTP is managed by environment variables. - - Policy: if `SMTP_HOST` is set, SMTP is treated as ENV-only. + Returns true when any SMTP ENV variable is set (used in Settings UI for hints). """ - @spec smtp_env_mode?() :: boolean() - def smtp_env_mode? do - smtp_host_env_set?() - end - - @doc """ - Returns missing required SMTP ENV keys for ENV-only mode warnings. - - Required in ENV-only mode: - - `SMTP_USERNAME` - - one of `SMTP_PASSWORD` or `SMTP_PASSWORD_FILE` - """ - @spec smtp_missing_required_env_keys() :: [String.t()] - def smtp_missing_required_env_keys do - if smtp_env_mode?() do - [] - |> maybe_add_missing("SMTP_USERNAME", smtp_username_env_set?()) - |> maybe_add_missing("SMTP_PASSWORD/SMTP_PASSWORD_FILE", smtp_password_env_set?()) - |> Enum.reverse() - else - [] - end + @spec smtp_env_configured?() :: boolean() + def smtp_env_configured? do + smtp_host_env_set?() or smtp_port_env_set?() or smtp_username_env_set?() or + smtp_password_env_set?() or smtp_ssl_env_set?() end @doc "Returns true if SMTP_HOST ENV is set." @@ -631,15 +590,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 @@ -665,18 +618,14 @@ defmodule Mv.Config do @spec mail_from_email_env_set?() :: boolean() def mail_from_email_env_set?, do: env_set?("MAIL_FROM_EMAIL") - defp parse_smtp_port_env(value) when is_binary(value) do - case Integer.parse(String.trim(value)) do - {port, _} when port > 0 -> port - _ -> nil + # Reads a plain string SMTP setting: ENV first, then Settings. + defp smtp_env_or_setting(env_key, setting_key) do + case System.get_env(env_key) do + nil -> get_from_settings(setting_key) + value -> trim_nil(value) end end - defp parse_smtp_port_env(_), do: nil - - defp maybe_add_missing(acc, _label, true), do: acc - defp maybe_add_missing(acc, label, false), do: [label | acc] - # Reads an integer setting attribute from Settings. defp get_from_settings_integer(key) do case Mv.Membership.get_settings() do 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..e5ac4e9 100644 --- a/lib/mv/mailer.ex +++ b/lib/mv/mailer.ex @@ -31,7 +31,6 @@ defmodule Mv.Mailer do import Swoosh.Email use Gettext, backend: MvWeb.Gettext, otp_app: :mv - alias Mv.Smtp.ConfigBuilder require Logger # Simple format check for test-email recipient only (e.g. allows a@b.c). Not for strict RFC validation. @@ -100,24 +99,32 @@ defmodule Mv.Mailer do """ @spec smtp_config() :: keyword() def smtp_config do - # In test we use Swoosh.Adapters.Test; do not override with SMTP opts or emails would not land in the test mailbox. - adapter = Application.get_env(:mv, __MODULE__, []) |> Keyword.get(:adapter) + if Mv.Config.smtp_configured?() and not boot_smtp_configured?() do + host = Mv.Config.smtp_host() + port = Mv.Config.smtp_port() || 587 + username = Mv.Config.smtp_username() + password = Mv.Config.smtp_password() + ssl_mode = Mv.Config.smtp_ssl() || "tls" - if Mv.Config.smtp_configured?() and not boot_smtp_configured?() and - adapter != Swoosh.Adapters.Test do verify_mode = if Application.get_env(:mv, :smtp_verify_peer, false), do: :verify_peer, else: :verify_none - ConfigBuilder.build_opts( - host: Mv.Config.smtp_host(), - port: Mv.Config.smtp_port() || 587, - username: Mv.Config.smtp_username(), - password: Mv.Config.smtp_password(), - ssl_mode: Mv.Config.smtp_ssl() || "tls", - verify_mode: verify_mode - ) + [ + adapter: Swoosh.Adapters.SMTP, + relay: host, + port: port, + ssl: ssl_mode == "ssl", + tls: if(ssl_mode == "tls", do: :always, else: :never), + auth: :always, + username: username, + password: password, + # tls_options: STARTTLS (587); sockopts: direct SSL (465). Verify from :smtp_verify_peer (ENV SMTP_VERIFY_PEER). + tls_options: [verify: verify_mode], + sockopts: [verify: verify_mode] + ] + |> Enum.reject(fn {_k, v} -> is_nil(v) end) else [] end @@ -190,4 +197,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_lookup.ex b/lib/mv/membership/custom_field_lookup.ex deleted file mode 100644 index 9d9b9f3..0000000 --- a/lib/mv/membership/custom_field_lookup.ex +++ /dev/null @@ -1,56 +0,0 @@ -defmodule Mv.Membership.CustomFieldLookup do - @moduledoc """ - Shared helper for loading custom fields by ID. - """ - - alias Mv.Constants - alias Mv.Membership - - @spec fetch_map_by_ids([String.t()], keyword()) :: map() - def fetch_map_by_ids(field_ids, opts \\ []) when is_list(field_ids) do - member_field_strings = Constants.member_fields() |> Enum.map(&Atom.to_string/1) - - custom_field_ids = - field_ids - |> Enum.uniq() - |> Enum.reject(&(&1 in member_field_strings)) - - if custom_field_ids == [] do - %{} - else - select = Keyword.get(opts, :select, [:id, :name, :value_type]) - - query = - Membership.CustomField - |> Ash.Query.select(select) - - read_opts = - [domain: Membership] - |> maybe_put_actor(opts) - |> maybe_put_authorize(opts) - - case Ash.read(query, read_opts) do - {:ok, fields} -> - allowed_ids = MapSet.new(custom_field_ids) - fields |> Enum.filter(&MapSet.member?(allowed_ids, &1.id)) |> Map.new(&{&1.id, &1}) - - {:error, _} -> - %{} - end - end - end - - defp maybe_put_actor(opts, read_opts) do - case Keyword.fetch(read_opts, :actor) do - {:ok, actor} -> Keyword.put(opts, :actor, actor) - :error -> opts - end - end - - defp maybe_put_authorize(opts, read_opts) do - case Keyword.fetch(read_opts, :authorize?) do - {:ok, authorize?} -> Keyword.put(opts, :authorize?, authorize?) - :error -> opts - end - end -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..00dcadf 100644 --- a/lib/mv/release.ex +++ b/lib/mv/release.ex @@ -6,8 +6,8 @@ defmodule Mv.Release do ## Tasks - `migrate/0` - Runs all pending Ecto migrations. - - `bootstrap_seeds_applied?/0` - Returns whether bootstrap was already applied (admin user exists). Used to skip re-running seeds. - - `run_seeds/0` - If bootstrap already applied, skips; otherwise runs bootstrap seeds (fee types, custom fields, roles, settings). Set `FORCE_SEEDS=true` to re-run seeds even when already applied. In production, set `RUN_DEV_SEEDS=true` to also run dev seeds (members, groups, sample data). + - `run_seeds/0` - Runs bootstrap seeds (fee types, custom fields, roles, settings). + In production, set `RUN_DEV_SEEDS=true` to also run dev seeds (members, groups, sample data). - `seed_admin/0` - Ensures an admin user exists from ENV (ADMIN_EMAIL, ADMIN_PASSWORD or ADMIN_PASSWORD_FILE). Idempotent; can be run on every deployment or via shell to update the admin password without redeploying. @@ -19,47 +19,22 @@ defmodule Mv.Release do alias Mv.Authorization.Role require Ash.Query - 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)) end end - @doc """ - Returns whether bootstrap seeds have already been applied (admin user exists). - - We check for the admin user (from ADMIN_EMAIL or default), not the Admin role, - because migrations may create the Admin role for the system actor. Only seeds - create the admin (login) user. Used to skip re-running seeds on subsequent starts. - Call only when the application is already started. - """ - def bootstrap_seeds_applied? do - admin_email = get_env("ADMIN_EMAIL", "admin@localhost") - - case User - |> Ash.Query.filter(email == ^admin_email) - |> Ash.read_one(authorize?: false, domain: Mv.Accounts) do - {:ok, %User{}} -> true - _ -> false - end - rescue - e -> - Logger.warning("Could not check seed status (#{inspect(e)}), assuming not applied.") - false - end - @doc """ Runs seed scripts so the database has required bootstrap data (and optionally dev data). - - Skips if bootstrap was already applied (admin user exists); set `FORCE_SEEDS=true` to override and re-run. - - If `RUN_DEV_SEEDS` env is set to `"true"`, also runs dev seeds (members, groups, sample data) - when bootstrap is run. + - Always runs bootstrap seeds (fee types, custom fields, roles, system user, settings). + - If `RUN_DEV_SEEDS` env is set to `"true"`, also runs dev seeds (members, groups, sample data). - Uses paths from the application's priv dir so it works in releases (no Mix). + Uses paths from the application's priv dir so it works in releases (no Mix). Idempotent. """ def run_seeds do case Application.ensure_all_started(@app) do @@ -67,32 +42,28 @@ defmodule Mv.Release do {:error, {app, reason}} -> raise "Failed to start #{inspect(app)}: #{inspect(reason)}" end - if bootstrap_seeds_applied?() and System.get_env("FORCE_SEEDS") != "true" do - IO.puts("Seeds already applied. Skipping. (Set FORCE_SEEDS=true to override)") - else - priv = :code.priv_dir(@app) - bootstrap_path = Path.join(priv, "repo/seeds_bootstrap.exs") - dev_path = Path.join(priv, "repo/seeds_dev.exs") + priv = :code.priv_dir(@app) + bootstrap_path = Path.join(priv, "repo/seeds_bootstrap.exs") + dev_path = Path.join(priv, "repo/seeds_dev.exs") - prev = Code.compiler_options() - _ = Code.compiler_options(ignore_module_conflict: true) + prev = Code.compiler_options() + Code.compiler_options(ignore_module_conflict: true) - try do - _ = Code.eval_file(bootstrap_path) - IO.puts("✅ Bootstrap seeds completed.") + try do + Code.eval_file(bootstrap_path) + IO.puts("✅ Bootstrap seeds completed.") - if System.get_env("RUN_DEV_SEEDS") == "true" do - _ = Code.eval_file(dev_path) - IO.puts("✅ Dev seeds completed.") - end - after - Code.compiler_options(prev) + if System.get_env("RUN_DEV_SEEDS") == "true" do + Code.eval_file(dev_path) + IO.puts("✅ Dev seeds completed.") end + after + Code.compiler_options(prev) end 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 +110,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 +160,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 +178,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/smtp/config_builder.ex b/lib/mv/smtp/config_builder.ex deleted file mode 100644 index 5018dff..0000000 --- a/lib/mv/smtp/config_builder.ex +++ /dev/null @@ -1,58 +0,0 @@ -defmodule Mv.Smtp.ConfigBuilder do - @moduledoc """ - Builds Swoosh/gen_smtp SMTP adapter options from connection parameters. - - Single source of truth for TLS/sockopts logic (port 587 vs 465): - - Port 587 (STARTTLS): `gen_tcp` is used first; `sockopts` must NOT contain `:verify`. - - Port 465 (implicit SSL): initial connection is `ssl:connect`; `sockopts` must contain `:verify`. - - Used by `config/runtime.exs` (boot-time ENV) and `Mv.Mailer.smtp_config/0` (Settings-only). - """ - - @doc """ - Builds the keyword list of Swoosh SMTP adapter options. - - Options (keyword list): - - `:host` (required) — relay hostname - - `:port` (required) — port number (e.g. 587 or 465) - - `:ssl_mode` (required) — `"tls"` or `"ssl"` - - `:verify_mode` (required) — `:verify_peer` or `:verify_none` - - `:username` (optional) - - `:password` (optional) - - Nil values are stripped from the result. - """ - @spec build_opts(keyword()) :: keyword() - def build_opts(opts) do - host = Keyword.fetch!(opts, :host) - port = Keyword.fetch!(opts, :port) - username = Keyword.get(opts, :username) - password = Keyword.get(opts, :password) - ssl_mode = Keyword.fetch!(opts, :ssl_mode) - verify_mode = Keyword.fetch!(opts, :verify_mode) - - base_opts = [ - adapter: Swoosh.Adapters.SMTP, - relay: host, - port: port, - username: username, - password: password, - ssl: ssl_mode == "ssl", - tls: if(ssl_mode == "tls", do: :always, else: :never), - auth: :always, - # tls_options: used for STARTTLS (587). For 465, gen_smtp uses sockopts for initial ssl:connect. - tls_options: [verify: verify_mode] - ] - - # Port 465: initial connection is ssl:connect; pass verify in sockopts. - # Port 587: initial connection is gen_tcp; sockopts must NOT contain verify (gen_tcp rejects it). - opts = - if ssl_mode == "ssl" do - Keyword.put(base_opts, :sockopts, verify: verify_mode) - else - base_opts - end - - Enum.reject(opts, fn {_k, v} -> is_nil(v) end) - 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/auth_overrides.ex b/lib/mv_web/auth_overrides.ex index 3aab0ed..5cab4d2 100644 --- a/lib/mv_web/auth_overrides.ex +++ b/lib/mv_web/auth_overrides.ex @@ -3,70 +3,52 @@ defmodule MvWeb.AuthOverrides do UI customizations for AshAuthentication Phoenix components. ## Overrides - - `SignIn` - Restricts form width and hides the library banner (title is rendered in SignInLive) - - `Banner` - Replaces default logo with text for reset/confirm pages - - `Flash` - Hides library flash (we use flash_group in root layout) + - `SignIn` - Restricts form width to prevent full-width display + - `Banner` - Replaces default logo with "Mitgliederverwaltung" text + - `HorizontalRule` - Translates "or" text to German ## Documentation For complete reference on available overrides, see: https://hexdocs.pm/ash_authentication_phoenix/ui-overrides.html """ use AshAuthentication.Phoenix.Overrides + use Gettext, backend: MvWeb.Gettext - # Avoid full-width for the Sign In Form. - # Banner is hidden because SignInLive renders its own locale-aware title. + # configure your UI overrides here + + # First argument to `override` is the component name you are overriding. + # The body contains any number of configurations you wish to override + # Below are some examples + + # For a complete reference, see https://hexdocs.pm/ash_authentication_phoenix/ui-overrides.html + + # override AshAuthentication.Phoenix.Components.Banner do + # set :image_url, "https://media.giphy.com/media/g7GKcSzwQfugw/giphy.gif" + # set :text_class, "bg-red-500" + # end + + # Avoid full-width for the Sign In Form override AshAuthentication.Phoenix.Components.SignIn do set :root_class, "md:min-w-md" - set :show_banner, false end - # Replace banner logo with text for reset/confirm pages (no image so link has discernible text). + # Replace banner logo with text (no image in light or dark so link has discernible text) override AshAuthentication.Phoenix.Components.Banner do set :text, "Mitgliederverwaltung" set :image_url, nil set :dark_image_url, nil end - # Hide AshAuthentication's Flash component since we use flash_group in root layout. - # This prevents duplicate flash messages. + # Translate the "or" in the horizontal rule (between password form and SSO). + # Uses auth domain so it respects the current locale (e.g. "oder" in German). + override AshAuthentication.Phoenix.Components.HorizontalRule do + set :text, dgettext("auth", "or") + end + + # Hide AshAuthentication's Flash component since we use flash_group in root layout + # This prevents duplicate flash messages override AshAuthentication.Phoenix.Components.Flash do set :message_class_info, "hidden" set :message_class_error, "hidden" end end - -defmodule MvWeb.AuthOverridesRegistrationDisabled do - @moduledoc """ - When direct registration is disabled in global settings, this override is - prepended in SignInLive so the Password component hides the "Need an account?" - toggle (register_toggle_text: nil disables the register link per library docs). - """ - use AshAuthentication.Phoenix.Overrides - - override AshAuthentication.Phoenix.Components.Password do - set :register_toggle_text, nil - end -end - -defmodule MvWeb.AuthOverridesDE do - @moduledoc """ - German locale-specific overrides for AshAuthentication Phoenix components. - - Prepended to the overrides list in SignInLive when the locale is "de". - Provides runtime-static German text for components that do not use - the `_gettext` mechanism (e.g. HorizontalRule renders its text directly), - and for submit buttons whose disable_text bypasses the POT extraction pipeline. - """ - use AshAuthentication.Phoenix.Overrides - - # HorizontalRule renders text without `_gettext`, so we need a static German string. - override AshAuthentication.Phoenix.Components.HorizontalRule do - set :text, "oder" - end - - # Registering ... disable-text is passed through _gettext but "Registering ..." - # has no dgettext source reference, so we supply the German string directly. - override AshAuthentication.Phoenix.Components.Password.RegisterForm do - set :disable_button_text, "Registrieren..." - end -end 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..11a60ef 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -63,11 +63,6 @@ defmodule MvWeb.CoreComponents do values: [:info, :error, :success, :warning], doc: "used for styling and flash lookup" - attr :auto_clear_ms, :integer, - default: nil, - doc: - "when set, flash is auto-dismissed after this many milliseconds (e.g. 5000 for success toasts)" - attr :rest, :global, doc: "the arbitrary HTML attributes to add to the flash container" slot :inner_block, doc: "the optional inner block that renders the flash message" @@ -79,9 +74,6 @@ defmodule MvWeb.CoreComponents do
    hide("##{@id}")} role="alert" class="pointer-events-auto" @@ -464,9 +456,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 +490,6 @@ defmodule MvWeb.CoreComponents do <.icon name={@icon} /> <% end %> {@button_label} - {render_slot(@trigger_badge)} - <.icon name="hero-chevron-down" class="size-4" />
      @@ -1051,13 +1018,6 @@ defmodule MvWeb.CoreComponents do has_click = col[:col_click] || @row_click classes = ["max-w-xs"] - classes = - if @sticky_first_col && col_idx == 0 do - ["sticky-first-col-cell sticky left-0 z-20" | classes] - else - classes - end - classes = if col_class == nil || (col_class && !String.contains?(col_class, "text-center")) do ["truncate" | classes] @@ -1072,7 +1032,7 @@ defmodule MvWeb.CoreComponents do classes end - # WCAG: no focus ring on the cell itself; sticky zebra rows show keyboard focus via CSS :has(:focus-visible) + # WCAG: no focus ring on the cell itself; row shows focus via focus-within classes = if @row_click && @first_row_click_col_idx == col_idx do [ @@ -1143,11 +1103,30 @@ defmodule MvWeb.CoreComponents do end end - # Returns CSS classes for table row selection styles. - # Hover/focus row highlighting is CSS-driven via [data-row-interactive] selectors in app.css. - # Sticky-first-column zebra tables use CSS accents and omit selected row ring classes. - defp table_row_tr_class(true, false), do: "ring-2 ring-inset ring-primary" - defp table_row_tr_class(_, _), do: "" + # Returns CSS classes for table row: hover/focus-within outline when row_click is set, + # and stronger selected outline when selected (WCAG: not color-only). + # Hover/focus-within are omitted for the selected row so the selected ring stays visible. + defp table_row_tr_class(row_click, selected?) do + has_row_click? = not is_nil(row_click) + base = [] + + base = + if has_row_click? and not selected?, + do: + base ++ + [ + "hover:ring-2", + "hover:ring-inset", + "hover:ring-base-content/10", + "focus-within:ring-2", + "focus-within:ring-inset", + "focus-within:ring-base-content/10" + ], + else: base + + base = if selected?, do: base ++ ["ring-2", "ring-inset", "ring-primary"], else: base + Enum.join(base, " ") + end defp table_th_aria_sort(col, sort_field, sort_order) do col_sort = Map.get(col, :sort_field) @@ -1316,41 +1295,6 @@ defmodule MvWeb.CoreComponents do """ end - @doc """ - Renders a theme toggle using DaisyUI swap (sun/moon with rotate effect). - - Wired to the theme script in root layout: checkbox uses `data-theme-toggle`, - root script syncs checked state (checked = dark) and listens for `phx:set-theme`. - Use in public header or sidebar. Optional `class` is applied to the wrapper. - """ - attr :class, :string, default: nil, doc: "Optional extra classes for the swap wrapper" - - def theme_swap(assigns) do - assigns = assign(assigns, :wrapper_class, assigns[:class]) - - ~H""" -
      - -
      - """ - end - @doc """ Renders a [Heroicon](https://heroicons.com). diff --git a/lib/mv_web/components/export_dropdown.ex b/lib/mv_web/components/export_dropdown.ex new file mode 100644 index 0000000..1e08168 --- /dev/null +++ b/lib/mv_web/components/export_dropdown.ex @@ -0,0 +1,110 @@ +defmodule MvWeb.Components.ExportDropdown do + @moduledoc """ + Export dropdown component for member export (CSV/PDF). + + Provides an accessible dropdown menu with CSV and PDF export options. + Uses the same export payload as the previous single-button export. + """ + 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 #{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) + + {:ok, socket} + end + + @impl true + def render(assigns) do + button_label = + gettext("Export") <> + " (" <> + if(assigns.selected_count == 0, + do: gettext("all"), + else: to_string(assigns.selected_count) + ) <> + ")" + + assigns = assign(assigns, :button_label, button_label) + + ~H""" +
      + <.dropdown_menu + id={"#{@id}-menu"} + button_label={@button_label} + icon="hero-arrow-down-tray" + open={@open} + phx_target={@myself} + menu_width="w-48" + menu_align="left" + button_class="btn-secondary gap-2" + testid="export-dropdown" + button_testid="export-dropdown-button" + menu_testid="export-dropdown-menu" + > +
    • +
      + + + + +
      • +
    • +
      + + + + +
      • + +
      + """ + 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/layouts.ex b/lib/mv_web/components/layouts.ex index 9aff23c..2979eb4 100644 --- a/lib/mv_web/components/layouts.ex +++ b/lib/mv_web/components/layouts.ex @@ -13,98 +13,6 @@ defmodule MvWeb.Layouts do embed_templates "layouts/*" - @doc """ - Builds the full browser tab title: "Mila", "Mila · Page", or "Mila · Page · Club". - Order is always: Mila · page title · club name. - Uses assigns[:club_name] and the short page label from assigns[:content_title] or - assigns[:page_title]. LiveViews should set content_title (same gettext as sidebar) - and then assign page_title to the result of this function so the client receives - the full title. - """ - def page_title_string(assigns) do - club = assigns[:club_name] - page = assigns[:content_title] || assigns[:page_title] - - parts = - [page, club] - |> Enum.filter(&(is_binary(&1) and String.trim(&1) != "")) - - if parts == [] do - "Mila" - else - "Mila · " <> Enum.join(parts, " · ") - end - end - - @doc """ - Assigns content_title (short label for heading; same gettext as sidebar) and - page_title (full browser tab title). Call from LiveView mount after club_name - is set (e.g. from on_mount). Returns the socket. - """ - def assign_page_title(socket, content_title) do - socket = assign(socket, :content_title, content_title) - assign(socket, :page_title, page_title_string(socket.assigns)) - end - - @doc """ - Renders the public (unauthenticated) page layout: header with logo + "Mitgliederverwaltung" left, - club name centered, language selector right; plus main content and flash group. Use for sign-in, join, and join-confirm pages so they - share the same chrome without the sidebar or authenticated layout logic. - - Pass optional `:club_name` from the parent (e.g. LiveView mount) to avoid a settings read in the component. - """ - attr :flash, :map, required: true, doc: "the map of flash messages" - - attr :club_name, :string, - default: nil, - doc: "optional; if set, avoids get_settings() in the component" - - slot :inner_block, required: true - - def public_page(assigns) do - club_name = - assigns[:club_name] || - case Mv.Membership.get_settings() do - {:ok, s} -> s.club_name || "Mitgliederverwaltung" - _ -> "Mitgliederverwaltung" - end - - assigns = assign(assigns, :club_name, club_name) - - ~H""" -
      -
      - Mila Logo - Mitgliederverwaltung -
      - - {@club_name} - -
      -
      - - - - <.theme_swap /> -
      -
      -
      -
      - {render_slot(@inner_block)} -
      -
      - <.flash_group flash={@flash} /> - """ - end - @doc """ Renders the app layout. Can be used with or without a current_user. When current_user is present, it will show the navigation bar. @@ -138,7 +46,7 @@ defmodule MvWeb.Layouts do # Single get_settings() for layout; derive club_name and join_form_enabled to avoid duplicate query. %{club_name: club_name, join_form_enabled: join_form_enabled} = get_layout_settings() - # NOTE: Unprocessed count runs on every page load when join form is enabled; consider + # TODO: unprocessed count runs on every page load when join form enabled; consider # loading only on navigation or caching briefly if performance becomes an issue. unprocessed_join_requests_count = get_unprocessed_join_requests_count(assigns.current_user, join_form_enabled) @@ -191,30 +99,24 @@ defmodule MvWeb.Layouts do <% else %> - -
      -
      - Mila Logo - Mitgliederverwaltung -
      - + +
      + Mila Logo + {@club_name} -
      -
      - - - - <.theme_swap /> -
      +
      + + +
      @@ -265,7 +167,7 @@ defmodule MvWeb.Layouts do aria-live="polite" class="z-50 toast toast-bottom toast-end flex flex-col gap-2 pointer-events-none" > - <.flash kind={:success} flash={@flash} auto_clear_ms={5000} /> + <.flash kind={:success} flash={@flash} /> <.flash kind={:warning} flash={@flash} /> <.flash kind={:info} flash={@flash} /> <.flash kind={:error} flash={@flash} /> diff --git a/lib/mv_web/components/layouts/root.html.heex b/lib/mv_web/components/layouts/root.html.heex index bb900aa..e107d5b 100644 --- a/lib/mv_web/components/layouts/root.html.heex +++ b/lib/mv_web/components/layouts/root.html.heex @@ -7,8 +7,8 @@ - <.live_title default="Mila"> - {page_title_string(assigns)} + <.live_title default="Mv" suffix=" · Phoenix Framework"> + {assigns[:page_title]} ") - - refute result =~ "
      {col[:label]} @@ -1025,13 +998,7 @@ defmodule MvWeb.CoreComponents do