diff --git a/.drone.yml b/.drone.yml index 483a08a..06db32b 100644 --- a/.drone.yml +++ b/.drone.yml @@ -4,7 +4,7 @@ name: check services: - name: postgres - image: docker.io/library/postgres:17.6 + image: docker.io/library/postgres:18.1 environment: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres @@ -57,7 +57,7 @@ steps: - mix gettext.extract --check-up-to-date - name: wait_for_postgres - image: docker.io/library/postgres:17.6 + image: docker.io/library/postgres:18.1 commands: # Wait for postgres to become available - | @@ -166,7 +166,7 @@ environment: steps: - name: renovate - image: renovate/renovate:41.173 + image: renovate/renovate:42.71 environment: RENOVATE_CONFIG_FILE: "renovate_backend_config.js" RENOVATE_TOKEN: diff --git a/.gitignore b/.gitignore index 9517a21..058543c 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,4 @@ npm-debug.log # Docker secrets directory (generated by `just init-secrets`) /secrets/ +notes.md diff --git a/.tool-versions b/.tool-versions index 98239f3..275206c 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,3 @@ elixir 1.18.3-otp-27 erlang 27.3.4 -just 1.43.1 +just 1.46.0 diff --git a/code_review_fixes_-_membership_fee_features_e886dc4b.plan.md b/code_review_fixes_-_membership_fee_features_e886dc4b.plan.md new file mode 100644 index 0000000..eebf419 --- /dev/null +++ b/code_review_fixes_-_membership_fee_features_e886dc4b.plan.md @@ -0,0 +1,318 @@ +--- +name: Code Review Fixes - Membership Fee Features +overview: Umsetzung der validen Code Review Punkte aus beiden Reviews mit Priorisierung nach Kritikalität. Fokus auf Transaktionssicherheit, Code-Qualität, Performance und UX-Verbesserungen. +todos: + - id: fix-after-action-tasks + content: "after_action mit Task.start → after_transaction + Task.Supervisor: Task.Supervisor zu application.ex hinzufügen, after_action Hooks in after_transaction umwandeln, Task.Supervisor.async_nolink verwenden" + status: pending + - id: reduce-code-duplication + content: "Code-Duplikation reduzieren: handle_cycle_generation/2 private Funktion extrahieren, alle drei Stellen (Create, Type Change, Date Change) verwenden" + status: pending + dependencies: + - fix-after-action-tasks + - id: fix-join-date-validation + content: "join_date Validierung: Entweder Validierung wieder hinzufügen (validate compare(:join_date, less_than_or_equal_to: &Date.utc_today/0)) oder Dokumentation anpassen" + status: pending + - id: fix-load-cycles-docs + content: "load_cycles_for_members: Entweder Dokumentation korrigieren (ehrlich machen) oder echte Filterung implementieren (z.B. nur letzte 2 Intervalle)" + status: pending + - id: fix-get-current-cycle-sort + content: "get_current_cycle nondeterministisch: Vor List.first() nach cycle_start sortieren (desc) in MembershipFeeHelpers.get_current_cycle" + status: pending + - id: fix-n1-query-member-count + content: "N+1 Query beheben: Aggregate auf MembershipFeeType definieren oder member_count einmalig vorab laden und in assigns cachen" + status: pending + - id: fix-assign-new-stale + content: "assign_new → assign: In MembershipFeesComponent.update/2 immer assign(:cycles, cycles) und assign(:available_fee_types, available_fee_types) setzen" + status: pending + - id: fix-regenerating-flag + content: "@regenerating auf true setzen: Direkt beim Event-Start in handle_event(\"regenerate_cycles\", ...) socket |> assign(:regenerating, true) setzen" + status: pending + - id: fix-create-cycle-parsing + content: "Create-cycle parsing Fix: Decimal.parse explizit behandeln und {:error, :invalid_amount} zurückgeben statt :error" + status: pending + - id: fix-delete-all-atomic + content: "Delete all cycles atomar: Bulk Delete Query verwenden (Ash bulk destroy oder Query-basiert) statt Enum.map" + status: pending + - id: improve-async-error-handling + content: "Fehlerbehandlung bei async Tasks: Strukturierte Error-Logs mit Context, optional Retry-Mechanismus oder Event-System für Benachrichtigung" + status: pending + - id: improve-format-currency + content: "format_currency Robustheit: Number.Currency verwenden oder robusteres Pattern Matching + Tests für Edge Cases (negative Zahlen, sehr große Zahlen)" + status: pending + - id: add-missing-typespecs + content: "Fehlende Typespecs: @spec für SetDefaultMembershipFeeType.change/3 hinzufügen" + status: pending + - id: fix-race-condition + content: "Potenzielle Race Condition: Prüfen ob Ash doppelte Auslösung verhindert, ggf. Logik anpassen (beide Änderungen in einem Hook zusammenfassen)" + status: pending + - id: extract-magic-values + content: "Magic Numbers/Strings: Application.get_env(:mv, :sql_sandbox, false) in Konstante/Helper extrahieren (z.B. Mv.Config.sql_sandbox?/0)" + status: pending + - id: fix-domain-consistency + content: "Domain-Konsistenz: Überall in MembershipFeesComponent domain: MembershipFees explizit angeben" + status: pending + - id: fix-test-helper + content: "Test-Helper Fix: create_cycle/3 Helper - Cleanup nur einmal im Setup oder gezielt nur auto-generierte löschen" + status: pending + - id: fix-date-utc-today-param + content: "Date.utc_today() Parameter: today Parameter durchgeben in get_cycle_status_for_member und Helper-Funktionen" + status: pending + - id: fix-ui-locale-input + content: "UI/Locale Input Fix: type=\"number\" → type=\"text\" + inputmode=\"decimal\" + serverseitig \",\" → \".\" normalisieren" + status: pending + - id: fix-delete-confirmation + content: "Delete-all-Confirmation robuster: String.trim() + case-insensitive Vergleich oder \"type DELETE\" Pattern" + status: pending + - id: fix-warning-state + content: "Warning-State Fix: Bei Decimal.parse(:error) explizit hide_amount_warning(socket) aufrufen" + status: pending + - id: fix-double-toggle + content: "Toggle entfernen: Toggle-Button im Spalten-Header entfernen (nur in Toolbar behalten)" + status: pending + - id: fix-format-consistency + content: "Format-Konsistenz: Inputs ebenfalls auf Komma ausrichten oder serverseitig normalisieren" + status: pending + dependencies: + - fix-ui-locale-input +--- + +# Code Review Fixes - Membership Fee Features + +## Kritische Probleme (Müssen vor Merge behoben werden) + +### 1. after_action mit Task.start - Transaktionsprobleme + +**Dateien:** `lib/membership/member.ex` (Zeilen 142, 279) + +**Problem:** `Task.start/1` wird innerhalb von `after_action` Hooks verwendet. `after_action` läuft innerhalb der DB-Transaktion, daher: + +- Tasks sehen möglicherweise noch nicht committed state +- Tasks werden auch bei Rollback gestartet +- Keine Supervision → Memory Leaks möglich + +**Lösung:** + +- `after_transaction` Hook verwenden (Ash Best Practice) +- `Task.Supervisor` zum Supervision Tree hinzufügen (`lib/mv/application.ex`) +- `Task.Supervisor.async_nolink/3` statt `Task.start/1` verwenden + +**Betroffene Stellen:** + +- Member Creation (Zeile 116-164) +- Join/Exit Date Change (Zeile 250-301) + +### 2. Code-Duplikation in Cycle-Generation-Logik + +**Datei:** `lib/membership/member.ex` + +**Problem:** Cycle-Generation-Logik ist dreimal dupliziert (Create, Type Change, Date Change) + +**Lösung:** Extrahiere in private Funktion `handle_cycle_generation/2` + +## Wichtige Probleme (Sollten behoben werden) + +### 3. join_date Validierung entfernt, aber Dokumentation behauptet Gegenteil + +**Datei:** `lib/membership/member.ex` (Zeile 27, 516-518) + +**Problem:** Dokumentation sagt "join_date not in future", aber Validierung fehlt + +**Lösung:** Dokumentation anpassen + +### 4. load_cycles_for_members overpromises + +**Datei:** `lib/mv_web/member_live/index/membership_fee_status.ex` (Zeile 36-40) + +**Problem:** Dokumentation sagt "Only loads the relevant cycle per member" und "Filters cycles at database level", aber lädt alle Cycles + +**Lösung:** echte Filterung implementieren (z.B. nur letzte 2 Intervalle) + +### 5. get_current_cycle nondeterministisch + +**Datei:** `lib/mv_web/helpers/membership_fee_helpers.ex` (Zeile 178-182) + +**Problem:** `List.first()` ohne explizite Sortierung → Ergebnis hängt von Reihenfolge ab + +**Lösung:** Vor `List.first()` nach `cycle_start` sortieren (desc) + +### 6. N+1 Query durch get_member_count + +**Datei:** `lib/mv_web/live/membership_fee_type_live/index.ex` (Zeile 134-140) + +**Problem:** `get_member_count/1` wird pro Row aufgerufen → N+1 Query + +**Lösung:** Aggregate auf MembershipFeeType definieren oder einmalig vorab laden + +### 7. assign_new kann stale werden + +**Datei:** `lib/mv_web/live/member_live/show/membership_fees_component.ex` (Zeile 402-403) + +**Problem:** `assign_new(:cycles, ...)` und `assign_new(:available_fee_types, ...)` werden nur gesetzt, wenn Assign noch nicht existiert + +**Lösung:** In `update/2` immer `assign(:cycles, cycles)` / `assign(:available_fee_types, available_fee_types)` setzen + +### 8. @regenerating wird nie auf true gesetzt + +**Datei:** `lib/mv_web/live/member_live/show/membership_fees_component.ex` (Zeile 526-561) + +**Problem:** `regenerating` wird nur auf `false` gesetzt, nie auf `true` → Button/Spinner werden nie disabled + +**Lösung:** Direkt beim Event-Start `socket |> assign(:regenerating, true)` setzen + +### 9. Create-cycle parsing: invalid amount zeigt falsche Fehlermeldung + +**Datei:** `lib/mv_web/live/member_live/show/membership_fees_component.ex` (Zeile 748-812) + +**Problem:** `Decimal.parse/1` gibt `:error` zurück, aber `with` behandelt es als `:error` → landet in "Invalid date format" Branch + +**Lösung:** Explizit `{:error, :invalid_amount}` zurückgeben: + +```elixir +amount = case Decimal.parse(amount_str) do + {d, _} -> {:ok, d} + :error -> {:error, :invalid_amount} +end +``` + +### 10. Delete all cycles: nicht atomar + +**Datei:** `lib/mv_web/live/member_live/show/membership_fees_component.ex` (Zeile 666-714) + +**Problem:** `Enum.map(cycles, &Ash.destroy/1)` → nicht atomar, teilweise gelöscht möglich + +**Lösung:** Bulk Delete Query verwenden (Ash bulk destroy oder Query-basiert) + +### 11. Fehlerbehandlung bei async Tasks + +**Datei:** `lib/membership/member.ex` + +**Problem:** Bei Fehlern in async Tasks wird nur geloggt, aber der Benutzer erhält keine Rückmeldung. Die Member-Aktion wird als erfolgreich zurückgegeben, auch wenn die Cycle-Generierung fehlschlägt. Keine Retry-Logik oder Monitoring. + +**Lösung:** + +- Für kritische Fälle: synchron ausführen oder Retry-Mechanismus implementieren +- Für nicht-kritische Fälle: Event-System für spätere Benachrichtigung +- Strukturierte Error-Logs mit Context +- Optional: Error-Tracking (Sentry, etc.) + +### 12. format_currency Robustheit + +**Datei:** `lib/mv_web/helpers/membership_fee_helpers.ex` (Zeilen 27-51) + +**Problem:** Die Funktion verwendet String-Manipulation für Formatierung. Edge Cases könnten problematisch sein (z.B. sehr große Zahlen, negative Werte). + +**Lösung:** + +- `Number.Currency` oder ähnliche Bibliothek verwenden +- Oder: Robusteres Pattern Matching für Edge Cases +- Tests für Edge Cases hinzufügen (negative Zahlen, sehr große Zahlen) + +### 13. Fehlende Typespecs + +**Datei:** `lib/membership/member/changes/set_default_membership_fee_type.ex` + +**Problem:** Keine `@spec` für die `change/3` Funktion. + +**Lösung:** Typespecs hinzufügen für bessere Dokumentation und Dialyzer-Support. + +### 14. Potenzielle Race Condition + +**Datei:** `lib/membership/member.ex` (Zeile 250-301) + +**Problem:** Wenn `join_date` und `exit_date` gleichzeitig geändert werden, könnte die Cycle-Generierung zweimal ausgelöst werden (einmal pro Änderung). + +**Lösung:** Prüfen, ob Ash dies bereits verhindert, oder Logik anpassen (z.B. beide Änderungen in einem Hook zusammenfassen). + +### 15. Magic Numbers/Strings + +**Problem:** `Application.get_env(:mv, :sql_sandbox, false)` wird mehrfach verwendet. + +**Lösung:** Extrahiere in Konstante oder Helper-Funktion (z.B. `Mv.Config.sql_sandbox?/0`). + +## Mittlere Probleme (Nice-to-have) + +### 16. Inconsistent use of domain + +**Datei:** `lib/mv_web/live/member_live/show/membership_fees_component.ex` (Zeile 819-821) + +**Problem:** Einige Actions verwenden `domain: MembershipFees`, andere nicht + +**Lösung:** Konsistent `domain` überall verwenden + +### 17. Tests: create_cycle/3 löscht jedes Mal alle Cycles + +**Datei:** `test/mv_web/member_live/index/membership_fee_status_test.exs` (Zeile 45-52) + +**Problem:** Helper löscht vor jedem Create alle Cycles → Tests prüfen nicht, was sie denken + +**Lösung:** Cleanup nur einmal im Setup oder gezielt nur auto-generierte löschen + +### 18. Tests/Design: Date.utc_today() macht Tests flaky + +**Problem:** Tests hängen von `Date.utc_today()` ab → nicht deterministisch + +**Lösung:** `today` Parameter durchgeben (z.B. `get_cycle_status_for_member(member, show_current, today \\ Date.utc_today())`) + +### 19. UI/Locale: input type="number" + Decimal/Komma + +**Problem:** `type="number"` funktioniert nicht zuverlässig mit Komma als Dezimaltrenner + +**Lösung:** `type="text"` + `inputmode="decimal"` + serverseitig "," → "." normalisieren + +### 20. Delete-all-Confirmation: String-Vergleich ist fragil + +**Datei:** `lib/mv_web/live/member_live/show/membership_fees_component.ex` (Zeile 296-298) + +**Problem:** String-Vergleich gegen `gettext("Yes")` und `"Yes"` → fragil bei Whitespace/Locale + +**Lösung:** `String.trim()` + case-insensitive Vergleich oder "type DELETE" Pattern + +### 21. MembershipFeeType Form: Warning-State kann hängen bleiben + +**Datei:** `lib/mv_web/live/membership_fee_type_live/form.ex` (Zeile 367-378) + +**Problem:** Bei `Decimal.parse(:error)` wird nur `socket` zurückgegeben → Warning kann stehen bleiben + +**Lösung:** Bei `:error` explizit `hide_amount_warning(socket)` aufrufen + +### 22. UI/UX: Toggle ist doppelt vorhanden + +**Datei:** `lib/mv_web/live/member_live/index.html.heex` (Zeile 45-72, 284-296) + +**Problem:** Toggle-Button sowohl in Toolbar als auch im Spalten-Header + +**Lösung:** Toggle im Spalten-Header entfernen (nur in Toolbar behalten) + +### 23. Konsistenz: format_currency vs Inputs + +**Problem:** `format_currency` formatiert deutsch (Komma), aber Inputs erwarten Punkt + +**Lösung:** Inputs ebenfalls auf Komma ausrichten oder serverseitig normalisieren + +## Implementierungsreihenfolge + +1. **Kritisch:** after_action → after_transaction + Task.Supervisor +2. **Kritisch:** Code-Duplikation reduzieren +3. **Wichtig:** join_date Validierung/Dokumentation +4. **Wichtig:** load_cycles_for_members Dokumentation/Implementierung +5. **Wichtig:** get_current_cycle Sortierung +6. **Wichtig:** N+1 Query beheben +7. **Wichtig:** assign_new → assign +8. **Wichtig:** @regenerating auf true setzen +9. **Wichtig:** Create-cycle parsing Fix +10. **Wichtig:** Delete all cycles atomar +11. **Wichtig:** Fehlerbehandlung bei async Tasks +12. **Wichtig:** format_currency Robustheit +13. **Wichtig:** Fehlende Typespecs +14. **Wichtig:** Potenzielle Race Condition prüfen/beheben +15. **Wichtig:** Magic Numbers/Strings extrahieren +16. **Mittel:** Domain-Konsistenz +17. **Mittel:** Test-Helper Fix +18. **Mittel:** Date.utc_today() Parameter +19. **Mittel:** UI/Locale Fixes +20. **Mittel:** String-Vergleich robuster +21. **Mittel:** Warning-State Fix +22. **Mittel:** Toggle entfernen +23. **Mittel:** Format-Konsistenz + diff --git a/config/config.exs b/config/config.exs index 17891e0..cc338b2 100644 --- a/config/config.exs +++ b/config/config.exs @@ -49,7 +49,7 @@ config :spark, config :mv, ecto_repos: [Mv.Repo], generators: [timestamp_type: :utc_datetime], - ash_domains: [Mv.Membership, Mv.Accounts] + ash_domains: [Mv.Membership, Mv.Accounts, Mv.MembershipFees, Mv.Authorization] # Configures the endpoint config :mv, MvWeb.Endpoint, @@ -95,7 +95,16 @@ config :tailwind, # Configures Elixir's Logger config :logger, :default_formatter, format: "$time $metadata[$level] $message\n", - metadata: [:request_id] + metadata: [ + :request_id, + :user_id, + :member_id, + :member_email, + :error, + :error_type, + :cycles_count, + :notifications_count + ] # Use Jason for JSON parsing in Phoenix config :phoenix, :json_library, Jason diff --git a/config/test.exs b/config/test.exs index 2c4d2ba..326694e 100644 --- a/config/test.exs +++ b/config/test.exs @@ -47,4 +47,5 @@ config :mv, :session_identifier, :unsafe config :mv, :require_token_presence_for_authentication, false # Enable SQL Sandbox for async LiveView tests +# This flag controls sync vs async behavior in CycleGenerator after_action hooks config :mv, :sql_sandbox, true diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index b4b7a1f..1ed863a 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -33,7 +33,7 @@ services: restart: unless-stopped db-prod: - image: postgres:16-alpine + image: postgres:18.1-alpine container_name: mv-prod-db environment: POSTGRES_USER: postgres diff --git a/docker-compose.yml b/docker-compose.yml index 56876f2..8621603 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,7 +4,7 @@ networks: services: db: - image: postgres:17.6-alpine + image: postgres:18.1-alpine environment: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres @@ -29,7 +29,7 @@ services: rauthy: container_name: rauthy-dev - image: ghcr.io/sebadob/rauthy:0.32.0 + image: ghcr.io/sebadob/rauthy:0.33.4 environment: - LOCAL_TEST=true - SMTP_URL=mailcrab diff --git a/docs/contributions-architecture.md b/docs/contributions-architecture.md deleted file mode 100644 index 3718a3b..0000000 --- a/docs/contributions-architecture.md +++ /dev/null @@ -1,653 +0,0 @@ -# Membership Contributions - Technical Architecture - -**Project:** Mila - Membership Management System -**Feature:** Membership Contribution Management -**Version:** 1.0 -**Last Updated:** 2025-11-27 -**Status:** Architecture Design - Ready for Implementation - ---- - -## Purpose - -This document defines the technical architecture for the Membership Contributions system. It focuses on architectural decisions, patterns, module structure, and integration points **without** concrete implementation details. - -**Related Documents:** -- [contributions-overview.md](./contributions-overview.md) - Business logic and requirements -- [database-schema-readme.md](./database-schema-readme.md) - Database documentation -- [database_schema.dbml](./database_schema.dbml) - Database schema definition - ---- - -## Table of Contents - -1. [Architecture Principles](#architecture-principles) -2. [Domain Structure](#domain-structure) -3. [Data Architecture](#data-architecture) -4. [Business Logic Architecture](#business-logic-architecture) -5. [Integration Points](#integration-points) -6. [Acceptance Criteria](#acceptance-criteria) -7. [Testing Strategy](#testing-strategy) -8. [Security Considerations](#security-considerations) -9. [Performance Considerations](#performance-considerations) - ---- - -## Architecture Principles - -### Core Design Decisions - -1. **Single Responsibility:** - - Each module has one clear responsibility - - Period generation separated from status management - - Calendar logic isolated in dedicated module - -2. **No Redundancy:** - - No `period_end` field (calculated from `period_start` + `interval`) - - No `interval_type` field (read from `contribution_type.interval`) - - Eliminates data inconsistencies - -3. **Immutability Where Important:** - - `contribution_type.interval` cannot be changed after creation - - Prevents complex migration scenarios - - Enforced via Ash change validation - -4. **Historical Accuracy:** - - `amount` stored per period for audit trail - - Enables tracking of contribution changes over time - - Old periods retain original amounts - -5. **Calendar-Based Periods:** - - All periods aligned to calendar boundaries - - Simplifies date calculations - - Predictable period generation - ---- - -## Domain Structure - -### Ash Domain: `Mv.Contributions` - -**Purpose:** Encapsulates all contribution-related resources and logic - -**Resources:** -- `ContributionType` - Contribution type definitions (admin-managed) -- `ContributionPeriod` - Individual contribution periods per member - -**Extensions:** -- Member resource extended with contribution fields - -### Module Organization - -``` -lib/ -├── contributions/ -│ ├── contributions.ex # Ash domain definition -│ ├── contribution_type.ex # ContributionType resource -│ ├── contribution_period.ex # ContributionPeriod resource -│ └── changes/ -│ ├── prevent_interval_change.ex # Validates interval immutability -│ ├── set_contribution_start_date.ex # Auto-sets start date -│ └── validate_same_interval.ex # Validates interval match on type change -├── mv/ -│ └── contributions/ -│ ├── period_generator.ex # Period generation algorithm -│ └── calendar_periods.ex # Calendar period calculations -└── membership/ - └── member.ex # Extended with contribution relationships -``` - -### Separation of Concerns - -**Domain Layer (Ash Resources):** -- Data validation -- Relationship management -- Policy enforcement -- Action definitions - -**Business Logic Layer (`Mv.Contributions`):** -- Period generation algorithm -- Calendar calculations -- Date boundary handling -- Status transitions - -**UI Layer (LiveView):** -- User interaction -- Display logic -- Authorization checks -- Form handling - ---- - -## Data Architecture - -### Database Schema Extensions - -**See:** [database-schema-readme.md](./database-schema-readme.md) and [database_schema.dbml](./database_schema.dbml) for complete schema documentation. - -### New Tables - -1. **`contribution_types`** - - Purpose: Define contribution types with fixed intervals - - Key Constraint: `interval` field immutable after creation - - Relationships: has_many members, has_many contribution_periods - -2. **`contribution_periods`** - - Purpose: Individual contribution periods for members - - Key Design: NO `period_end` or `interval_type` fields (calculated) - - Relationships: belongs_to member, belongs_to contribution_type - - Composite uniqueness: One period per member per period_start - -### Member Table Extensions - -**Fields Added:** -- `contribution_type_id` (FK, NOT NULL with default from settings) -- `contribution_start_date` (Date, nullable) - -**Existing Fields Used:** -- `joined_at` - For calculating contribution start -- `left_at` - For limiting period generation -- These fields must remain member fields and should not be replaced by custom fields in the future - -### Settings Integration - -**Global Settings:** -- `contributions.include_joining_period` (Boolean) -- `contributions.default_contribution_type_id` (UUID) - -**Storage:** Existing settings mechanism (TBD: dedicated table or configuration resource) - -### Foreign Key Behaviors - -| Relationship | On Delete | Rationale | -|--------------|-----------|-----------| -| `contribution_periods.member_id → members.id` | CASCADE | Remove periods when member deleted | -| `contribution_periods.contribution_type_id → contribution_types.id` | RESTRICT | Prevent type deletion if periods exist | -| `members.contribution_type_id → contribution_types.id` | RESTRICT | Prevent type deletion if assigned to members | - ---- - -## Business Logic Architecture - -### Period Generation System - -**Component:** `Mv.Contributions.PeriodGenerator` - -**Responsibilities:** -- Calculate which periods should exist for a member -- Generate missing periods -- Respect contribution_start_date and left_at boundaries -- Skip existing periods (idempotent) - -**Triggers:** -1. Member contribution type assigned (via Ash change) -2. Member created with contribution type (via Ash change) -3. Scheduled job runs (daily/weekly cron) -4. Admin manual regeneration (UI action) - -**Algorithm Steps:** -1. Retrieve member with contribution_type and dates -2. Determine first period start (based on contribution_start_date) -3. Calculate all period starts from first to today (or left_at) -4. Query existing periods for member -5. Generate missing periods with current contribution_type.amount -6. Insert new periods (batch operation) - -**Edge Case Handling:** -- If contribution_start_date is NULL: Calculate from joined_at + global setting -- If left_at is set: Stop generation at left_at -- If contribution_type changes: Handled separately by regeneration logic - -### Calendar Period Calculations - -**Component:** `Mv.Contributions.CalendarPeriods` - -**Responsibilities:** -- Calculate period boundaries based on interval type -- Determine current period -- Determine last completed period -- Calculate period_end from period_start + interval - -**Functions (high-level):** -- `calculate_period_start/3` - Given date and interval, find period start -- `calculate_period_end/2` - Given period_start and interval, calculate end -- `next_period_start/2` - Given period_start and interval, find next -- `is_current_period?/2` - Check if period contains today -- `is_last_completed_period?/2` - Check if period just ended - -**Interval Logic:** -- **Monthly:** Start = 1st of month, End = last day of month -- **Quarterly:** Start = 1st of quarter (Jan/Apr/Jul/Oct), End = last day of quarter -- **Half-yearly:** Start = 1st of half (Jan/Jul), End = last day of half -- **Yearly:** Start = Jan 1st, End = Dec 31st - -### Status Management - -**Component:** Ash actions on `ContributionPeriod` - -**Status Transitions:** -- Simple state machine: unpaid ↔ paid ↔ suspended -- No complex validation (all transitions allowed) -- Permissions checked via Ash policies - -**Actions Required:** -- `mark_as_paid` - Set status to :paid -- `mark_as_suspended` - Set status to :suspended -- `mark_as_unpaid` - Set status to :unpaid (error correction) - -**Bulk Operations:** -- `bulk_mark_as_paid` - Mark multiple periods as paid (efficiency) - - low priority, can be a future issue - -### Contribution Type Change Handling - -**Component:** Ash change on `Member.contribution_type_id` - -**Validation:** -- Check if new type has same interval as old type -- If different: Reject change (MVP constraint) -- If same: Allow change - -**Side Effects on Allowed Change:** -1. Keep all existing periods unchanged -2. Find future unpaid periods -3. Delete future unpaid periods -4. Regenerate periods with new contribution_type_id and amount - -**Implementation Pattern:** -- Use Ash change module to validate -- Use after_action hook to trigger regeneration -- Use transaction to ensure atomicity - ---- - -## Integration Points - -### Member Resource Integration - -**Extension Points:** -1. Add fields via migration -2. Add relationships (belongs_to, has_many) -3. Add calculations (current_period_status, overdue_count) -4. Add changes (auto-set contribution_start_date, validate interval) - -**Backward Compatibility:** -- New fields nullable or with defaults -- Existing members get default contribution type from settings -- No breaking changes to existing member functionality - -### Settings System Integration - -**Requirements:** -- Store two global settings -- Provide UI for admin to modify -- Default values if not set -- Validation (e.g., default_contribution_type_id must exist) - -**Access Pattern:** -- Read settings during period generation -- Read settings during member creation -- Write settings only via admin UI - -### Permission System Integration - -**See:** [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) - -**Required Permissions:** -- `ContributionType.create/update/destroy` - Admin only -- `ContributionType.read` - Admin, Treasurer, Board -- `ContributionPeriod.update` (status changes) - Admin, Treasurer -- `ContributionPeriod.read` - Admin, Treasurer, Board, Own member - -**Policy Patterns:** -- Use existing HasPermission check -- Leverage existing roles (Admin, Kassenwart) -- Member can read own periods (linked via member_id) - -### LiveView Integration - -**New LiveViews Required:** -1. ContributionType index/form (admin) -2. ContributionPeriod table component (member detail view) -3. Settings form section (admin) -4. Member list column (contribution status) - -**Existing LiveViews to Extend:** -- Member detail view: Add contributions section -- Member list view: Add status column -- Settings page: Add contributions section - -**Authorization Helpers:** -- Use existing `can?/3` helper for UI conditionals -- Check permissions before showing actions - ---- - -## Acceptance Criteria - -### ContributionType Resource - -**AC-CT-1:** Admin can create contribution type with name, amount, interval, description -**AC-CT-2:** Interval field is immutable after creation (validation error on change attempt) -**AC-CT-3:** Admin can update name, amount, description (but not interval) -**AC-CT-4:** Cannot delete contribution type if assigned to members -**AC-CT-5:** Cannot delete contribution type if periods exist referencing it -**AC-CT-6:** Interval must be one of: monthly, quarterly, half_yearly, yearly - -### ContributionPeriod Resource - -**AC-CP-1:** Period has period_start, status, amount, notes, member_id, contribution_type_id -**AC-CP-2:** Period_end is calculated, not stored -**AC-CP-3:** Status defaults to :unpaid -**AC-CP-4:** One period per member per period_start (uniqueness constraint) -**AC-CP-5:** Amount is set at generation time from contribution_type.amount -**AC-CP-6:** Periods cascade delete when member deleted -**AC-CP-7:** Admin/Treasurer can change status -**AC-CP-8:** Member can read own periods - -### Member Extensions - -**AC-M-1:** Member has contribution_type_id field (NOT NULL with default) -**AC-M-2:** Member has contribution_start_date field (nullable) -**AC-M-3:** New members get default contribution type from global setting -**AC-M-4:** contribution_start_date auto-set based on joined_at and global setting -**AC-M-5:** Admin can manually override contribution_start_date -**AC-M-6:** Cannot change to contribution type with different interval (MVP) - -### Period Generation - -**AC-PG-1:** Periods generated when member gets contribution type -**AC-PG-2:** Periods generated when member created (via change hook) -**AC-PG-3:** Scheduled job generates missing periods daily -**AC-PG-4:** Generation respects contribution_start_date -**AC-PG-5:** Generation stops at left_at if member exited -**AC-PG-6:** Generation is idempotent (skips existing periods) -**AC-PG-7:** Periods align to calendar boundaries (1st of month/quarter/half/year) -**AC-PG-8:** Amount comes from contribution_type at generation time - -### Calendar Logic - -**AC-CL-1:** Monthly periods: 1st to last day of month -**AC-CL-2:** Quarterly periods: 1st of Jan/Apr/Jul/Oct to last day of quarter -**AC-CL-3:** Half-yearly periods: 1st of Jan/Jul to last day of half -**AC-CL-4:** Yearly periods: Jan 1 to Dec 31 -**AC-CL-5:** Period_end calculated correctly for all interval types -**AC-CL-6:** Current period determined correctly based on today's date -**AC-CL-7:** Last completed period determined correctly - -### Contribution Type Change - -**AC-TC-1:** Can change to type with same interval -**AC-TC-2:** Cannot change to type with different interval (error message) -**AC-TC-3:** On allowed change: future unpaid periods regenerated -**AC-TC-4:** On allowed change: paid/suspended periods unchanged -**AC-TC-5:** On allowed change: amount updated to new type's amount -**AC-TC-6:** Change is atomic (transaction) - -### Settings - -**AC-S-1:** Global setting: include_joining_period (boolean, default true) -**AC-S-2:** Global setting: default_contribution_type_id (UUID, required) -**AC-S-3:** Admin can modify settings via UI -**AC-S-4:** Settings validated (e.g., default type must exist) -**AC-S-5:** Settings applied to new members immediately - -### UI - Member List - -**AC-UI-ML-1:** New column shows contribution status -**AC-UI-ML-2:** Default: Shows last completed period status -**AC-UI-ML-3:** Optional: Toggle to show current period status -**AC-UI-ML-4:** Color coding: green (paid), red (unpaid), gray (suspended) -**AC-UI-ML-5:** Filter: Unpaid in last period -**AC-UI-ML-6:** Filter: Unpaid in current period - -### UI - Member Detail - -**AC-UI-MD-1:** Contributions section shows all periods -**AC-UI-MD-2:** Table columns: Period, Interval, Amount, Status, Actions -**AC-UI-MD-3:** Checkbox per period for bulk marking (low prio) -**AC-UI-MD-4:** "Mark selected as paid" button -**AC-UI-MD-5:** Dropdown to change contribution type (same interval only) -**AC-UI-MD-6:** Warning if different interval selected -**AC-UI-MD-7:** Only show actions if user has permission - -### UI - Contribution Types Admin - -**AC-UI-CTA-1:** List all contribution types -**AC-UI-CTA-2:** Show: Name, Amount, Interval, Member count -**AC-UI-CTA-3:** Create new contribution type form -**AC-UI-CTA-4:** Edit form: Name, Amount, Description editable -**AC-UI-CTA-5:** Edit form: Interval grayed out (not editable) -**AC-UI-CTA-6:** Warning on amount change (explain impact) -**AC-UI-CTA-7:** Cannot delete if members assigned -**AC-UI-CTA-8:** Only admin can access - -### UI - Settings Admin - -**AC-UI-SA-1:** Contributions section in settings -**AC-UI-SA-2:** Dropdown to select default contribution type -**AC-UI-SA-3:** Checkbox: Include joining period -**AC-UI-SA-4:** Explanatory text with examples -**AC-UI-SA-5:** Save button with validation - ---- - -## Testing Strategy - -### Unit Testing - -**Period Generator Tests:** -- Correct period_start calculation for all interval types -- Correct period count from start to end date -- Respects contribution_start_date boundary -- Respects left_at boundary -- Skips existing periods (idempotent) -- Handles edge dates (year boundaries, leap years) - -**Calendar Periods Tests:** -- Period boundaries correct for all intervals -- Period_end calculation correct -- Current period detection -- Last completed period detection -- Next period calculation - -**Validation Tests:** -- Interval immutability enforced -- Same interval validation on type change -- Status transitions allowed -- Uniqueness constraints enforced - -### Integration Testing - -**Period Generation Flow:** -- Member creation triggers generation -- Type assignment triggers generation -- Type change regenerates future periods -- Scheduled job generates missing periods -- Left member stops generation - -**Status Management Flow:** -- Mark single period as paid -- Bulk mark multiple periods (low prio) -- Status transitions work -- Permissions enforced - -**Contribution Type Management:** -- Create type -- Update amount (regeneration triggered) -- Cannot update interval -- Cannot delete if in use - -### LiveView Testing - -**Member List:** -- Status column displays correctly -- Toggle between last/current works -- Filters work correctly -- Color coding applied - -**Member Detail:** -- Periods table displays all periods -- Checkboxes work -- Bulk marking works (low prio) -- Type change validation works -- Actions only shown with permission - -**Admin UI:** -- Type CRUD works -- Settings save correctly -- Validations display errors -- Only authorized users can access - -### Edge Case Testing - -**Interval Change Attempt:** -- Error message displayed -- No data modified -- User can cancel/choose different type - -**Exit with Unpaid:** -- Warning shown -- Option to suspend offered -- Exit completes correctly - -**Amount Change:** -- Warning displayed -- Only future unpaid regenerated -- Historical periods unchanged - -**Date Boundaries:** -- Today = period start handled -- Today = period end handled -- Leap year handled - -### Performance Testing - -**Period Generation:** -- Generate 10 years of monthly periods: < 100ms -- Generate for 1000 members: < 5 seconds -- Idempotent check efficient (no full scan) - -**Member List Query:** -- With status column: < 200ms for 1000 members -- Filters applied efficiently -- No N+1 queries - ---- - -## Security Considerations - -### Authorization - -**Permissions Required:** -- ContributionType management: Admin only -- ContributionPeriod status changes: Admin + Treasurer -- View all periods: Admin + Treasurer + Board -- View own periods: All authenticated users - -**Policy Enforcement:** -- All actions protected by Ash policies -- UI shows/hides based on permissions -- Backend validates permissions (never trust UI alone) - -### Data Integrity - -**Validation Layers:** -1. Database constraints (NOT NULL, UNIQUE, CHECK) -2. Ash validations (business rules) -3. UI validations (user experience) - -**Immutability Protection:** -- Interval change prevented at multiple layers -- Period amounts immutable (audit trail) -- Settings changes logged (future) - -### Audit Trail - -**Tracked Information:** -- Period status changes (who, when) - future enhancement -- Type amount changes (implicit via period amounts) -- Member type assignments (via timestamps) - ---- - -## Performance Considerations - -### Database Indexes - -**Required Indexes:** -- `contribution_periods(member_id)` - For member period lookups -- `contribution_periods(contribution_type_id)` - For type queries -- `contribution_periods(status)` - For unpaid filters -- `contribution_periods(period_start)` - For date range queries -- `contribution_periods(member_id, period_start)` - Composite unique index -- `members(contribution_type_id)` - For type membership count - -### Query Optimization - -**Preloading:** -- Load contribution_type with periods (avoid N+1) -- Load periods when displaying member detail -- Use Ash's load for efficient preloading - -**Calculated Fields:** -- period_end calculated on-demand (not stored) -- current_period_status calculated when needed -- Use Ash calculations for lazy evaluation - -**Pagination:** -- Period list paginated if > 50 periods -- Member list already paginated - -### Caching Strategy - -**No caching needed in MVP:** -- Contribution types rarely change -- Period queries are fast -- Settings read infrequently - -**Future caching if needed:** -- Cache settings in application memory -- Cache contribution types list -- Invalidate on change - -### Scheduled Job Performance - -**Period Generation Job:** -- Run daily or weekly (not hourly) -- Batch members (process 100 at a time) -- Skip members with no changes -- Log failures for retry - ---- - -## Future Enhancements - -### Phase 2: Interval Change Support - -**Architecture Changes:** -- Add logic to handle period overlaps -- Calculate prorata amounts if needed -- More complex validation -- Migration path for existing periods - -### Phase 3: Payment Details - -**Architecture Changes:** -- Add PaymentTransaction resource -- Link transactions to periods -- Support multiple payments per period -- Reconciliation logic - -### Phase 4: vereinfacht.digital Integration - -**Architecture Changes:** -- External API client module -- Webhook handling for transactions -- Automatic matching logic -- Manual review interface - ---- - -**End of Architecture Document** - diff --git a/docs/csv-member-import-v1.md b/docs/csv-member-import-v1.md new file mode 100644 index 0000000..2bdbe69 --- /dev/null +++ b/docs/csv-member-import-v1.md @@ -0,0 +1,666 @@ +# CSV Member Import v1 - Implementation Plan + +**Version:** 1.0 +**Date:** 2025-01-XX +**Status:** Ready for Implementation +**Related Documents:** +- [Feature Roadmap](./feature-roadmap.md) - Overall feature planning + +--- + +## Table of Contents + +- [Overview & Scope](#overview--scope) +- [UX Flow](#ux-flow) +- [CSV Specification](#csv-specification) +- [Technical Design Notes](#technical-design-notes) +- [Implementation Issues](#implementation-issues) +- [Rollout & Risks](#rollout--risks) + +--- + +## Overview & Scope + +### What We're Building + +A **basic CSV member import feature** that allows administrators to upload a CSV file and import new members into the system. This is a **v1 minimal implementation** focused on establishing the import structure without advanced features. + +**Core Functionality (v1 Minimal):** +- Upload CSV file via LiveView file upload +- Parse CSV with bilingual header support for core member fields (English/German) +- Auto-detect delimiter (`;` or `,`) using header recognition +- Map CSV columns to core member fields (`first_name`, `last_name`, `email`, `street`, `postal_code`, `city`) +- **Import custom field values** - Map CSV columns to existing custom fields by name (unknown custom field columns will be ignored with a warning) +- Validate each row (required field: `email`) +- Create members via Ash resource (one-by-one, **no background jobs**, processed in chunks of 200 rows via LiveView messages) +- Display import results: success count, error count, and error details +- Provide static CSV templates (EN/DE) + +**Key Constraints (v1):** +- ✅ **Admin-only feature** +- ✅ **No upsert** (create only) +- ✅ **No deduplication** (duplicate emails fail and show as errors) +- ✅ **No mapping wizard** (fixed header mapping via bilingual variants) +- ✅ **No background jobs** (progress via LiveView `handle_info`) +- ✅ **Best-effort import** (row-by-row, no rollback) +- ✅ **UI-only error display** (no error CSV export) +- ✅ **Safety limits** (10 MB, 1,000 rows, chunks of 200) + +### Out of Scope (v1) + +**Deferred to Future Versions:** +- ❌ Upsert/update existing members +- ❌ Advanced deduplication strategies +- ❌ Column mapping wizard UI +- ❌ Background job processing (Oban/GenStage) +- ❌ Transactional all-or-nothing import +- ❌ Error CSV export/download +- ❌ Batch validation preview before import +- ❌ Dynamic template generation +- ❌ Import history/audit log +- ❌ Import templates for other entities + +--- + +## UX Flow + +### Access & Location + +**Entry Point:** +- **Location:** Global Settings page (`/settings`) +- **UI Element:** New section "Import Members (CSV)" below "Custom Fields" section +- **Access Control:** Admin-only (enforced at LiveView event level, not entire `/settings` route) + +### User Journey + +1. **Navigate to Global Settings** +2. **Access Import Section** + - **Important notice:** Custom fields should be created in Mila before importing CSV files with custom field columns (unknown columns will be ignored with a warning) + - Upload area (drag & drop or file picker) + - Template download links (English / German) + - Help text explaining CSV format and custom field requirements +3. **Ensure Custom Fields Exist (if importing custom fields)** + - Navigate to Custom Fields section and create required custom fields + - Note the name/identifier for each custom field (used as CSV header) +4. **Download Template (Optional)** +5. **Prepare CSV File** + - Include custom field columns using the custom field name as header (e.g., `membership_number`, `birth_date`) +6. **Upload CSV** +7. **Start Import** + - Runs server-side via LiveView messages (may take up to ~30 seconds for large files) + - Warning messages if custom field columns reference non-existent custom fields (columns will be ignored) +8. **View Results** + - Success count + - Error count + - First 50 errors, each with: + - **CSV line number** (header is line 1, first data record begins at line 2) + - Error message + - Field name (if applicable) + +### Error Handling + +- **File too large:** Flash error before upload starts +- **Too many rows:** Flash error before import starts +- **Invalid CSV format:** Error shown in results +- **Partial success:** Results show both success and error counts + +--- + +## CSV Specification + +### Delimiter + +**Recommended:** Semicolon (`;`) +**Supported:** `;` and `,` + +**Auto-Detection (Header Recognition):** +- Remove UTF-8 BOM *first* +- Extract header record and try parsing with both delimiters +- For each delimiter, count how many recognized headers are present (via normalized variants) +- Choose delimiter with higher recognition; prefer `;` if tied +- If neither yields recognized headers, default to `;` + +### Quoting Rules + +- Fields may be quoted with double quotes (`"`) +- Escaped quotes: `""` inside quoted field represents a single `"` +- **v1 assumption:** CSV records do **not** contain embedded newlines inside quoted fields. (If they do, parsing may fail or line numbers may be inaccurate.) + +### Column Headers + +**v1 Supported Fields:** + +**Core Member Fields:** +- `first_name` / `Vorname` (optional) +- `last_name` / `Nachname` (optional) +- `email` / `E-Mail` (required) +- `street` / `Straße` (optional) +- `postal_code` / `PLZ` / `Postleitzahl` (optional) +- `city` / `Stadt` (optional) + +**Custom Fields:** +- Any custom field column using the custom field's **name** as the header (e.g., `membership_number`, `birth_date`) +- **Important:** Custom fields must be created in Mila before importing. The CSV header must match the custom field name exactly (same normalization as member fields). +- **Behavior:** If the CSV contains custom field columns that don't exist in Mila, a warning message will be shown and those columns will be ignored during import. + +**Member Field Header Mapping:** + +| Canonical Field | English Variants | German Variants | +|---|---|---| +| `first_name` | `first_name`, `firstname` | `Vorname`, `vorname` | +| `last_name` | `last_name`, `lastname`, `surname` | `Nachname`, `nachname`, `Familienname` | +| `email` | `email`, `e-mail`, `e_mail` | `E-Mail`, `e-mail`, `e_mail` | +| `street` | `street`, `address` | `Straße`, `strasse`, `Strasse` | +| `postal_code` | `postal_code`, `zip`, `postcode` | `PLZ`, `plz`, `Postleitzahl`, `postleitzahl` | +| `city` | `city`, `town` | `Stadt`, `stadt`, `Ort` | + +**Header Normalization (used consistently for both input headers AND mapping variants):** +- Trim whitespace +- Convert to lowercase +- Normalize Unicode: `ß` → `ss` (e.g., `Straße` → `strasse`) +- Replace hyphens/whitespace with underscores: `E-Mail` → `e_mail`, `phone number` → `phone_number` +- Collapse multiple underscores: `e__mail` → `e_mail` +- Case-insensitive matching + +**Unknown columns:** ignored (no error) + +**Required fields:** `email` + +**Custom Field Columns:** +- Custom field columns are identified by matching the normalized CSV header to the custom field `name` (not slug) +- Same normalization rules apply as for member fields (trim, lowercase, Unicode normalization, underscore replacement) +- Unknown custom field columns (non-existent names) will be ignored with a warning message + +### CSV Template Files + +**Location:** +- `priv/static/templates/member_import_en.csv` +- `priv/static/templates/member_import_de.csv` + +**Content:** +- Header row with required + common optional fields +- **Note:** Custom field columns are not included in templates by default (users add them based on their custom field configuration) +- One example row +- Uses semicolon delimiter (`;`) +- UTF-8 encoding **with BOM** (Excel compatibility) + +**Template Access:** +- Templates are static files in `priv/static/templates/` +- Served at: + - `/templates/member_import_en.csv` + - `/templates/member_import_de.csv` +- In LiveView, link them using Phoenix static path helpers (e.g. `~p` or `Routes.static_path/2`, depending on Phoenix version). + +### File Limits + +- **Max file size:** 10 MB +- **Max rows:** 1,000 rows (excluding header) +- **Processing:** chunks of 200 (via LiveView messages) +- **Encoding:** UTF-8 (BOM handled) + +--- + +## Technical Design Notes + +### Architecture Overview + +``` +┌─────────────────┐ +│ LiveView UI │ (GlobalSettingsLive or component) +│ - Upload area │ +│ - Progress │ +│ - Results │ +└────────┬────────┘ + │ prepare + ▼ +┌─────────────────────────────┐ +│ Import Service │ (Mv.Membership.Import.MemberCSV) +│ - parse + map + limit checks│ -> returns import_state +│ - process_chunk(chunk) │ -> returns chunk results +└────────┬────────────────────┘ + │ create + ▼ +┌─────────────────┐ +│ Ash Resource │ (Mv.Membership.Member) +│ - Create │ +└─────────────────┘ +``` + +### Technology Stack + +- **Phoenix LiveView:** file upload via `allow_upload/3` +- **NimbleCSV:** CSV parsing (add explicit dependency if missing) +- **Ash Resource:** member creation via `Membership.create_member/1` +- **Gettext:** bilingual UI/error messages + +### Module Structure + +**New Modules:** +- `lib/mv/membership/import/member_csv.ex` - import orchestration + chunk processing + custom field handling +- `lib/mv/membership/import/csv_parser.ex` - delimiter detection + parsing + BOM handling +- `lib/mv/membership/import/header_mapper.ex` - normalization + header mapping (core fields + custom fields) + +**Modified Modules:** +- `lib/mv_web/live/global_settings_live.ex` - render import section, handle upload/events/messages + +### Data Flow + +1. **Upload:** LiveView receives file via `allow_upload` +2. **Consume:** `consume_uploaded_entries/3` reads file content +3. **Prepare:** `MemberCSV.prepare/2` + - Strip BOM + - Detect delimiter (header recognition) + - Parse header + rows + - Map headers to canonical fields (core member fields) + - **Query existing custom fields and map custom field columns by name** (using same normalization as member fields) + - **Warn about unknown custom field columns** (non-existent names will be ignored with warning) + - Early abort if required headers missing + - Row count check + - Return `import_state` containing chunks, column_map, and custom_field_map +4. **Process:** LiveView drives chunk processing via `handle_info` + - For each chunk: validate + create member + create custom field values + collect errors +5. **Results:** LiveView shows progress + final summary + +### Types & Key Consistency + +- **Raw CSV parsing:** returns headers as list of strings, and rows **with csv line numbers** +- **Header mapping:** operates on normalized strings; mapping table variants are normalized once +- **Ash attrs:** built as atom-keyed map (`%{first_name: ..., ...}`) + +### Error Model + +```elixir +%{ + csv_line_number: 5, # physical line number in the CSV file + field: :email, # optional + message: "is not a valid email" +} +``` + +### CSV Line Numbers (Important) + +To keep error reporting user-friendly and accurate, **row errors must reference the physical line number in the original file**, even if empty lines are skipped. + +**Design decision:** the parser returns rows as: + +```elixir +rows :: [{csv_line_number :: pos_integer(), row_map :: map()}] +``` + +Downstream logic must **not** recompute line numbers from row indexes. + +### Authorization + +**Enforcement points:** +1. **LiveView event level:** check admin permission in `handle_event("start_import", ...)` +2. **UI level:** render import section only for admin users +3. **Static templates:** public assets (no authorization needed) + +Use `Mv.Authorization.PermissionSets` (preferred) instead of hard-coded string checks where possible. + +### Safety Limits + +- File size enforced by `allow_upload` (`max_file_size`) +- Row count enforced in `MemberCSV.prepare/2` before processing starts +- Chunking is done via **LiveView `handle_info` loop** (sequential, cooperative scheduling) + +--- + +## Implementation Issues + +### Issue #1: CSV Specification & Static Template Files + +**Dependencies:** None + +**Goal:** Define CSV contract and add static templates. + +**Tasks:** +- [ ] Finalize header mapping variants +- [ ] Document normalization rules +- [ ] Document delimiter detection strategy +- [ ] Create templates in `priv/static/templates/` (UTF-8 with BOM) +- [ ] Document template URLs and how to link them from LiveView +- [ ] Document line number semantics (physical CSV line numbers) + +**Definition of Done:** +- [ ] Templates open cleanly in Excel/LibreOffice +- [ ] CSV spec section complete + +--- + +### Issue #2: Import Service Module Skeleton + +**Dependencies:** None + +**Goal:** Create service API and error types. + +**API (recommended):** +- `prepare/2` — parse + map + limit checks, returns import_state +- `process_chunk/3` — process one chunk (pure-ish), returns per-chunk results + +**Tasks:** +- [ ] Create `lib/mv/membership/import/member_csv.ex` +- [ ] Define public function: `prepare/2 (file_content, opts \\ [])` +- [ ] Define public function: `process_chunk/3 (chunk_rows_with_lines, column_map, opts \\ [])` +- [ ] Define error struct: `%MemberCSV.Error{csv_line_number: integer, field: atom | nil, message: String.t}` +- [ ] Document module + API + +--- + +### Issue #3: CSV Parsing + Delimiter Auto-Detection + BOM Handling + +**Dependencies:** Issue #2 + +**Goal:** Parse CSV robustly with correct delimiter detection and BOM handling. + +**Tasks:** +- [ ] Verify/add NimbleCSV dependency (`{:nimble_csv, "~> 1.0"}`) +- [ ] Create `lib/mv/membership/import/csv_parser.ex` +- [ ] Implement `strip_bom/1` and apply it **before** any header handling +- [ ] Handle `\r\n` and `\n` line endings (trim `\r` on header record) +- [ ] Detect delimiter via header recognition (try `;` and `,`) +- [ ] Parse CSV and return: + - `headers :: [String.t()]` + - `rows :: [{csv_line_number, [String.t()]}]` or directly `[{csv_line_number, row_map}]` +- [ ] Skip completely empty records (but preserve correct physical line numbers) +- [ ] Return `{:ok, headers, rows}` or `{:error, reason}` + +**Definition of Done:** +- [ ] BOM handling works (Excel exports) +- [ ] Delimiter detection works reliably +- [ ] Rows carry correct `csv_line_number` + +--- + +### Issue #4: Header Normalization + Per-Header Mapping (No Language Detection) + +**Dependencies:** Issue #3 + +**Goal:** Map each header individually to canonical fields (normalized comparison). + +**Tasks:** +- [ ] Create `lib/mv/membership/import/header_mapper.ex` +- [ ] Implement `normalize_header/1` +- [ ] Normalize mapping variants once and compare normalized strings +- [ ] Build `column_map` (canonical field -> column index) +- [ ] **Early abort if required headers missing** (`email`) +- [ ] Ignore unknown columns (member fields only) +- [ ] **Separate custom field column detection** (by name, with normalization) + +**Definition of Done:** +- [ ] English/German headers map correctly +- [ ] Missing required columns fails fast + +--- + +### Issue #5: Validation (Required Fields) + Error Formatting + +**Dependencies:** Issue #4 + +**Goal:** Validate each row and return structured, translatable errors. + +**Tasks:** +- [ ] Implement `validate_row/3 (row_map, csv_line_number, opts)` +- [ ] Required field presence (`email`) +- [ ] Email format validation (EctoCommons.EmailValidator) +- [ ] Trim values before validation +- [ ] Gettext-backed error messages + +--- + +### Issue #6: Persistence via Ash Create + Per-Row Error Capture (Chunked Processing) + +**Dependencies:** Issue #5 + +**Goal:** Create members and capture errors per row with correct CSV line numbers. + +**Tasks:** +- [ ] Implement `process_chunk/3` in service: + - Input: `[{csv_line_number, row_map}]` + - Validate + create sequentially + - Collect counts + first 50 errors (per import overall; LiveView enforces cap across chunks) +- [ ] Implement Ash error formatter helper: + - Convert `Ash.Error.Invalid` into `%MemberCSV.Error{}` + - Prefer field-level errors where possible (attach `field` atom) + - Handle unique email constraint error as user-friendly message +- [ ] Map row_map to Ash attrs (`%{first_name: ..., ...}`) + +**Important:** **Do not recompute line numbers** in this layer—use the ones provided by the parser. + +--- + +### Issue #7: Admin Global Settings LiveView UI (Upload + Start Import + Results + Template Links) + +**Dependencies:** Issue #6 + +**Goal:** UI section with upload, progress, results, and template links. + +**Tasks:** +- [ ] Render import section only for admins +- [ ] **Add prominent UI notice about custom fields:** + - Display alert/info box: "Custom fields must be created in Mila before importing CSV files with custom field columns" + - Explain: "Use the custom field name as the CSV column header (same normalization as member fields applies)" + - Add link to custom fields management section +- [ ] Configure `allow_upload/3`: + - `.csv` only, `max_entries: 1`, `max_file_size: 10MB`, `auto_upload: false` +- [ ] `handle_event("start_import", ...)`: + - Admin permission check + - Consume upload -> read file content + - Call `MemberCSV.prepare/2` + - Store `import_state` in assigns (chunks + column_map + metadata) + - Initialize progress assigns + - `send(self(), {:process_chunk, 0})` +- [ ] `handle_info({:process_chunk, idx}, socket)`: + - Fetch chunk from `import_state` + - Call `MemberCSV.process_chunk/3` + - Merge counts/errors into progress assigns (cap errors at 50 overall) + - Schedule next chunk (or finish and show results) +- [ ] Results UI: + - Success count + - Failure count + - Error list (line number + message + field) + - **Warning messages for unknown custom field columns** (non-existent names) shown in results + +**Template links:** +- Link `/templates/member_import_en.csv` and `/templates/member_import_de.csv` via Phoenix static path helpers. + +--- + +### Issue #8: Authorization + Limits + +**Dependencies:** None (can be parallelized) + +**Goal:** Ensure admin-only access and enforce limits. + +**Tasks:** +- [ ] Admin check in start import event handler +- [ ] File size enforced in upload config +- [ ] Row limit enforced in `MemberCSV.prepare/2` (max_rows from config) +- [ ] Configuration: + ```elixir + config :mv, csv_import: [ + max_file_size_mb: 10, + max_rows: 1000 + ] + ``` + +--- + +### Issue #9: End-to-End LiveView Tests + Fixtures + +**Dependencies:** Issue #7 and #8 + +**Tasks:** +- [ ] Fixtures: + - valid EN/DE (core fields only) + - valid with custom fields + - invalid + - unknown custom field name (non-existent, should show warning) + - too many rows (1,001) + - BOM + `;` delimiter fixture + - fixture with empty line(s) to validate correct line numbers +- [ ] LiveView tests: + - admin sees section, non-admin does not + - upload + start import + - success + error rendering + - row limit + file size errors + - custom field import success + - custom field import warning (non-existent name, column ignored) + +--- + +### Issue #10: Documentation Polish (Inline Help Text + Docs) + +**Dependencies:** Issue #9 + +**Tasks:** +- [ ] UI help text + translations +- [ ] CHANGELOG entry +- [ ] Ensure moduledocs/docs + +--- + +### Issue #11: Custom Field Import + +**Dependencies:** Issue #6 (Persistence) + +**Priority:** High (Core v1 Feature) + +**Goal:** Support importing custom field values from CSV columns. Custom fields should exist in Mila before import for best results. + +**Important Requirements:** +- **Custom fields should be created in Mila first** - Unknown custom field columns will be ignored with a warning message +- CSV headers for custom fields must match the custom field **name** exactly (same normalization as member fields applies) +- Custom field values are validated according to the custom field type (string, integer, boolean, date, email) +- Unknown custom field columns (non-existent names) will be ignored with a warning - import continues + +**Tasks:** +- [ ] Extend `header_mapper.ex` to detect custom field columns by name (using same normalization as member fields) +- [ ] Query existing custom fields during `prepare/2` to map custom field columns +- [ ] Collect unknown custom field columns and add warning messages (don't fail import) +- [ ] Map custom field CSV values to `CustomFieldValue` creation in `process_chunk/3` +- [ ] Handle custom field type validation (string, integer, boolean, date, email) +- [ ] Create `CustomFieldValue` records linked to members during import +- [ ] Update error messages to include custom field validation errors +- [ ] Add UI help text explaining custom field requirements: + - "Custom fields must be created in Mila before importing" + - "Use the custom field name as the CSV column header (same normalization as member fields)" + - Link to custom fields management section +- [ ] Update CSV templates documentation to explain custom field columns +- [ ] Add tests for custom field import (valid, invalid name, type validation, warning for unknown) + +**Definition of Done:** +- [ ] Custom field columns are recognized by name (with normalization) +- [ ] Warning messages shown for unknown custom field columns (import continues) +- [ ] Custom field values are created and linked to members +- [ ] Type validation works for all custom field types +- [ ] UI clearly explains custom field requirements +- [ ] Tests cover custom field import scenarios (including warning for unknown names) + +--- + +## Rollout & Risks + +### Rollout Strategy +- Dev → Staging → Production (with anonymized real-world CSV tests) + +### Risks & Mitigations + +| Risk | Impact | Likelihood | Mitigation | +|---|---:|---:|---| +| Large import timeout | High | Medium | 10 MB + 1,000 rows, chunking via `handle_info` | +| Encoding issues | Medium | Medium | BOM stripping, templates with BOM | +| Invalid CSV format | Medium | High | Clear errors + templates | +| Duplicate emails | Low | High | Ash constraint error -> user-friendly message | +| Performance (no background jobs) | Medium | Low | Small limits, sequential chunk processing | +| Admin access bypass | High | Low | Event-level auth + UI hiding | +| Data corruption | High | Low | Per-row validation + best-effort | + +--- + +## Appendix + +### Module File Structure + +``` +lib/ +├── mv/ +│ └── membership/ +│ └── import/ +│ ├── member_csv.ex # prepare + process_chunk +│ ├── csv_parser.ex # delimiter detection + parsing + BOM handling +│ └── header_mapper.ex # normalization + header mapping +└── mv_web/ + └── live/ + └── global_settings_live.ex # add import section + LV message loop + +priv/ +└── static/ + └── templates/ + ├── member_import_en.csv + └── member_import_de.csv + +test/ +├── mv/ +│ └── membership/ +│ └── import/ +│ ├── member_csv_test.exs +│ ├── csv_parser_test.exs +│ └── header_mapper_test.exs +└── fixtures/ + ├── member_import_en.csv + ├── member_import_de.csv + ├── member_import_invalid.csv + ├── member_import_large.csv + └── member_import_empty_lines.csv +``` + +### Example Usage (LiveView) + +```elixir +def handle_event("start_import", _params, socket) do + assert_admin!(socket.assigns.current_user) + + [{_name, content}] = + consume_uploaded_entries(socket, :csv_file, fn %{path: path}, _entry -> + {:ok, File.read!(path)} + end) + + case Mv.Membership.Import.MemberCSV.prepare(content) do + {:ok, import_state} -> + socket = + socket + |> assign(:import_state, import_state) + |> assign(:import_progress, %{processed: 0, inserted: 0, failed: 0, errors: []}) + |> assign(:importing?, true) + + send(self(), {:process_chunk, 0}) + {:noreply, socket} + + {:error, reason} -> + {:noreply, put_flash(socket, :error, reason)} + end +end + +def handle_info({:process_chunk, idx}, socket) do + %{chunks: chunks, column_map: column_map} = socket.assigns.import_state + + case Enum.at(chunks, idx) do + nil -> + {:noreply, assign(socket, importing?: false)} + + chunk_rows_with_lines -> + {:ok, chunk_result} = + Mv.Membership.Import.MemberCSV.process_chunk(chunk_rows_with_lines, column_map) + + socket = merge_progress(socket, chunk_result) # caps errors at 50 overall + + send(self(), {:process_chunk, idx + 1}) + {:noreply, socket} + end +end +``` + +--- + +**End of Implementation Plan** \ No newline at end of file diff --git a/docs/custom-fields-search-performance.md b/docs/custom-fields-search-performance.md new file mode 100644 index 0000000..3987c85 --- /dev/null +++ b/docs/custom-fields-search-performance.md @@ -0,0 +1,243 @@ +# Performance Analysis: Custom Fields in Search Vector + +## Current Implementation + +The search vector includes custom field values via database triggers that: +1. Aggregate all custom field values for a member +2. Extract values from JSONB format +3. Add them to the search_vector with weight 'C' + +## Performance Considerations + +### 1. Trigger Performance on Member Updates + +**Current Implementation:** +- `members_search_vector_trigger()` executes a subquery on every INSERT/UPDATE: + ```sql + SELECT string_agg(...) FROM custom_field_values WHERE member_id = NEW.id + ``` + +**Performance Impact:** +- ✅ **Good:** Index on `member_id` exists (`custom_field_values_member_id_idx`) +- ✅ **Good:** Subquery only runs for the affected member +- ⚠️ **Potential Issue:** With many custom fields per member (e.g., 50+), aggregation could be slower +- ⚠️ **Potential Issue:** JSONB extraction (`value->>'_union_value'`) is relatively fast but adds overhead + +**Expected Performance:** +- **Small scale (< 10 custom fields per member):** Negligible impact (< 5ms per operation) +- **Medium scale (10-30 custom fields):** Minor impact (5-20ms per operation) +- **Large scale (30+ custom fields):** Noticeable impact (20-50ms+ per operation) + +### 2. Trigger Performance on Custom Field Value Changes + +**Current Implementation:** +- `update_member_search_vector_from_custom_field_value()` executes on every INSERT/UPDATE/DELETE on `custom_field_values` +- **Optimized:** Only fetches required member fields (not full record) to reduce overhead +- **Optimized:** Skips re-aggregation on UPDATE if value hasn't actually changed +- Aggregates all custom field values, then updates member search_vector + +**Performance Impact:** +- ✅ **Good:** Index on `member_id` ensures fast lookup +- ✅ **Optimized:** Only required fields are fetched (first_name, last_name, email, etc.) instead of full record +- ✅ **Optimized:** UPDATE operations that don't change the value skip expensive re-aggregation (early return) +- ⚠️ **Note:** Re-aggregation is still necessary when values change (required for search_vector consistency) +- ⚠️ **Critical:** Bulk operations (e.g., importing 1000 members with custom fields) will trigger this for each row + +**Expected Performance:** +- **Single operation (value changed):** 3-10ms per custom field value change (improved from 5-15ms) +- **Single operation (value unchanged):** <1ms (early return, no aggregation) +- **Bulk operations:** Could be slow (consider disabling trigger temporarily) + +### 3. Search Vector Size + +**Current Constraints:** +- String values: max 10,000 characters per custom field +- No limit on number of custom fields per member +- tsvector has no explicit size limit, but very large vectors can cause issues + +**Potential Issues:** +- **Theoretical maximum:** If a member has 100 custom fields with 10,000 char strings each, the aggregated text could be ~1MB +- **Practical concern:** Very large search vectors (> 100KB) can slow down: + - Index updates (GIN index maintenance) + - Search queries (tsvector operations) + - Trigger execution time + +**Recommendation:** +- Monitor search_vector size in production +- Consider limiting total custom field content per member if needed +- PostgreSQL can handle large tsvectors, but performance degrades gradually + +### 4. Initial Migration Performance + +**Current Implementation:** +- Updates ALL members in a single transaction: + ```sql + UPDATE members m SET search_vector = ... (subquery for each member) + ``` + +**Performance Impact:** +- ⚠️ **Potential Issue:** With 10,000+ members, this could take minutes +- ⚠️ **Potential Issue:** Single transaction locks the members table +- ⚠️ **Potential Issue:** If migration fails, entire rollback required + +**Recommendation:** +- For large datasets (> 10,000 members), consider: + - Batch updates (e.g., 1000 members at a time) + - Run during maintenance window + - Monitor progress + +### 5. Search Query Performance + +**Current Implementation:** +- Full-text search uses GIN index on `search_vector` (fast) +- Additional LIKE queries on `custom_field_values` for substring matching: + ```sql + EXISTS (SELECT 1 FROM custom_field_values WHERE member_id = id AND ... LIKE ...) + ``` + +**Performance Impact:** +- ✅ **Good:** GIN index on `search_vector` is very fast +- ⚠️ **Potential Issue:** LIKE queries on JSONB are not indexed (sequential scan) +- ⚠️ **Potential Issue:** EXISTS subquery runs for every search, even if search_vector match is found +- ⚠️ **Potential Issue:** With many custom fields, the LIKE queries could be slow + +**Expected Performance:** +- **With GIN index match:** Very fast (< 10ms for typical queries) +- **Without GIN index match (fallback to LIKE):** Slower (10-100ms depending on data size) +- **Worst case:** Sequential scan of all custom_field_values for all members + +## Recommendations + +### Short-term (Current Implementation) + +1. **Monitor Performance:** + - Add logging for trigger execution time + - Monitor search_vector size distribution + - Track search query performance + +2. **Index Verification:** + - Ensure `custom_field_values_member_id_idx` exists and is used + - Verify GIN index on `search_vector` is maintained + +3. **Bulk Operations:** + - For bulk imports, consider temporarily disabling the custom_field_values trigger + - Re-enable and update search_vectors in batch after import + +### Medium-term Optimizations + +1. **✅ Optimize Trigger Function (FULLY IMPLEMENTED):** + - ✅ Only fetch required member fields instead of full record (reduces overhead) + - ✅ Skip re-aggregation on UPDATE if value hasn't actually changed (early return optimization) + +2. **Limit Search Vector Size:** + - Truncate very long custom field values (e.g., first 1000 chars) + - Add warning if aggregated text exceeds threshold + +3. **Optimize LIKE Queries:** + - Consider adding a generated column for searchable text + - Or use a materialized view for custom field search + +### Long-term Considerations + +1. **Alternative Approaches:** + - Separate search index table for custom fields + - Use Elasticsearch or similar for advanced search + - Materialized view for search optimization + +2. **Scaling Strategy:** + - If performance becomes an issue with 100+ custom fields per member: + - Consider limiting which custom fields are searchable + - Use a separate search service + - Implement search result caching + +## Performance Benchmarks (Estimated) + +Based on typical PostgreSQL performance: + +| Scenario | Members | Custom Fields/Member | Expected Impact | +|----------|---------|---------------------|-----------------| +| Small | < 1,000 | < 10 | Negligible (< 5ms per operation) | +| Medium | 1,000-10,000 | 10-30 | Minor (5-20ms per operation) | +| Large | 10,000-100,000 | 30-50 | Noticeable (20-50ms per operation) | +| Very Large | > 100,000 | 50+ | Significant (50-200ms+ per operation) | + +## Monitoring Queries + +```sql +-- Check search_vector size distribution +SELECT + pg_size_pretty(octet_length(search_vector::text)) as size, + COUNT(*) as member_count +FROM members +WHERE search_vector IS NOT NULL +GROUP BY octet_length(search_vector::text) +ORDER BY octet_length(search_vector::text) DESC +LIMIT 20; + +-- Check average custom fields per member +SELECT + AVG(custom_field_count) as avg_custom_fields, + MAX(custom_field_count) as max_custom_fields +FROM ( + SELECT member_id, COUNT(*) as custom_field_count + FROM custom_field_values + GROUP BY member_id +) subq; + +-- Check trigger execution time (requires pg_stat_statements) +SELECT + mean_exec_time, + calls, + query +FROM pg_stat_statements +WHERE query LIKE '%members_search_vector_trigger%' +ORDER BY mean_exec_time DESC; +``` + +## Code Quality Improvements (Post-Review) + +### Refactored Search Implementation + +The search query has been refactored for better maintainability and clarity: + +**Before:** Single large OR-chain with mixed search types (hard to maintain) + +**After:** Modular functions grouped by search type: +- `build_fts_filter/1` - Full-text search (highest priority, fastest) +- `build_substring_filter/2` - Substring matching on structured fields +- `build_custom_field_filter/1` - Custom field value search (JSONB LIKE) +- `build_fuzzy_filter/2` - Trigram/fuzzy matching for names and streets + +**Benefits:** +- ✅ Clear separation of concerns +- ✅ Easier to maintain and test +- ✅ Better documentation of search priority +- ✅ Easier to optimize individual search types + +**Search Priority Order:** +1. **FTS (Full-Text Search)** - Fastest, uses GIN index on search_vector +2. **Substring** - For structured fields (postal_code, phone_number, etc.) +3. **Custom Fields** - JSONB LIKE queries (fallback for substring matching) +4. **Fuzzy Matching** - Trigram similarity for names and streets + +## Conclusion + +The current implementation is **well-optimized for typical use cases** (< 30 custom fields per member, < 10,000 members). For larger scales, monitoring and potential optimizations may be needed. + +**Key Strengths:** +- Indexed lookups (member_id index) +- Efficient GIN index for search +- Trigger-based automatic updates +- Modular, maintainable search code structure + +**Key Weaknesses:** +- LIKE queries on JSONB (not indexed) +- Re-aggregation on every custom field change (necessary for consistency) +- Potential size issues with many/large custom fields +- Substring searches (contains/ILIKE) not index-optimized + +**Recent Optimizations:** +- ✅ Trigger function optimized to fetch only required fields (reduces overhead by ~30-50%) +- ✅ Early return on UPDATE when value hasn't changed (skips expensive re-aggregation, <1ms vs 3-10ms) +- ✅ Improved performance for custom field value updates (3-10ms vs 5-15ms when value changes) + diff --git a/docs/database-schema-readme.md b/docs/database-schema-readme.md index 1644f2a..6457db5 100644 --- a/docs/database-schema-readme.md +++ b/docs/database-schema-readme.md @@ -168,9 +168,16 @@ Member (1) → (N) Properties ### Weighted Fields - **Weight A (highest):** first_name, last_name - **Weight B:** email, notes -- **Weight C:** phone_number, city, street, house_number, postal_code +- **Weight C:** phone_number, city, street, house_number, postal_code, custom_field_values - **Weight D (lowest):** join_date, exit_date +### Custom Field Values in Search +Custom field values are automatically included in the search vector: +- All custom field values (string, integer, boolean, date, email) are aggregated and added to the search vector +- Values are converted to text format for indexing +- Custom field values receive weight 'C' (same as phone_number, city, etc.) +- The search vector is automatically updated when custom field values are created, updated, or deleted via database triggers + ### Usage Example ```sql SELECT * FROM members diff --git a/docs/database_schema.dbml b/docs/database_schema.dbml index b620830..f97463e 100644 --- a/docs/database_schema.dbml +++ b/docs/database_schema.dbml @@ -6,8 +6,8 @@ // - https://dbdocs.io // - VS Code Extensions: "DBML Language" or "dbdiagram.io" // -// Version: 1.2 -// Last Updated: 2025-11-13 +// Version: 1.3 +// Last Updated: 2025-12-11 Project mila_membership_management { database_type: 'PostgreSQL' @@ -27,6 +27,7 @@ Project mila_membership_management { ## Domains: - **Accounts**: User authentication and session management - **Membership**: Club member data and custom fields + - **MembershipFees**: Membership fee types and billing cycles ## Required PostgreSQL Extensions: - uuid-ossp (UUID generation) @@ -132,6 +133,8 @@ Table members { house_number text [null, note: 'House number'] postal_code text [null, note: '5-digit German postal code'] search_vector tsvector [null, note: 'Full-text search index (auto-generated)'] + membership_fee_type_id uuid [null, note: 'FK to membership_fee_types - assigned fee type'] + membership_fee_start_date date [null, note: 'Date from which membership fees should be calculated'] indexes { email [unique, name: 'members_unique_email_index'] @@ -146,6 +149,7 @@ Table members { last_name [name: 'members_last_name_idx', note: 'B-tree index for name sorting'] join_date [name: 'members_join_date_idx', note: 'B-tree index for date filters'] (paid) [name: 'members_paid_idx', type: btree, note: 'Partial index WHERE paid IS NOT NULL'] + membership_fee_type_id [name: 'members_membership_fee_type_id_index', note: 'B-tree index for fee type lookups'] } Note: ''' @@ -178,6 +182,8 @@ Table members { **Relationships:** - Optional 1:1 with users (0..1 ↔ 0..1) - authentication account - 1:N with custom_field_values (custom dynamic fields) + - Optional N:1 with membership_fee_types - assigned fee type + - 1:N with membership_fee_cycles - billing history **Validation Rules:** - first_name, last_name: min 1 character @@ -281,6 +287,98 @@ Table custom_fields { ''' } +// ============================================ +// MEMBERSHIP_FEES DOMAIN +// ============================================ + +Table membership_fee_types { + id uuid [pk, not null, default: `uuid_generate_v7()`, note: 'UUIDv7 primary key'] + name text [not null, unique, note: 'Unique name for the fee type (e.g., "Standard", "Reduced")'] + amount numeric(10,2) [not null, note: 'Fee amount in default currency (CHECK: >= 0)'] + interval text [not null, note: 'Billing interval (CHECK: IN monthly, quarterly, half_yearly, yearly) - immutable'] + description text [null, note: 'Optional description for the fee type'] + + indexes { + name [unique, name: 'membership_fee_types_unique_name_index'] + } + + Note: ''' + **Membership Fee Type Definitions** + + Defines the different types of membership fees with fixed billing intervals. + + **Attributes:** + - `name`: Unique identifier for the fee type + - `amount`: Default fee amount (stored per cycle for audit trail) + - `interval`: Billing cycle - immutable after creation + - `description`: Optional documentation + + **Interval Values:** + - `monthly`: 1st to last day of month + - `quarterly`: 1st of Jan/Apr/Jul/Oct to last day of quarter + - `half_yearly`: 1st of Jan/Jul to last day of half + - `yearly`: Jan 1 to Dec 31 + + **Immutability:** + The `interval` field cannot be changed after creation to prevent + complex migration scenarios. Create a new fee type to change intervals. + + **Relationships:** + - 1:N with members - members assigned to this fee type + - 1:N with membership_fee_cycles - all cycles using this fee type + + **Deletion Behavior:** + - ON DELETE RESTRICT: Cannot delete if members or cycles reference it + ''' +} + +Table membership_fee_cycles { + id uuid [pk, not null, default: `uuid_generate_v7()`, note: 'UUIDv7 primary key'] + cycle_start date [not null, note: 'Start date of the billing cycle'] + amount numeric(10,2) [not null, note: 'Fee amount for this cycle (CHECK: >= 0)'] + status text [not null, default: 'unpaid', note: 'Payment status (CHECK: IN unpaid, paid, suspended)'] + notes text [null, note: 'Optional notes for this cycle'] + member_id uuid [not null, note: 'FK to members - the member this cycle belongs to'] + membership_fee_type_id uuid [not null, note: 'FK to membership_fee_types - fee type for this cycle'] + + indexes { + member_id [name: 'membership_fee_cycles_member_id_index'] + membership_fee_type_id [name: 'membership_fee_cycles_membership_fee_type_id_index'] + status [name: 'membership_fee_cycles_status_index'] + cycle_start [name: 'membership_fee_cycles_cycle_start_index'] + (member_id, cycle_start) [unique, name: 'membership_fee_cycles_unique_cycle_per_member_index', note: 'One cycle per member per cycle_start'] + } + + Note: ''' + **Individual Membership Fee Cycles** + + Represents a single billing cycle for a member with payment tracking. + + **Design Decisions:** + - `cycle_end` is NOT stored - calculated from cycle_start + interval + - `amount` is stored per cycle to preserve historical values when fee type amount changes + - Cycles are aligned to calendar boundaries + + **Status Values:** + - `unpaid`: Payment pending (default) + - `paid`: Payment received + - `suspended`: Payment suspended (e.g., hardship case) + + **Constraints:** + - Unique: One cycle per member per cycle_start date + - member_id: Required (belongs_to) + - membership_fee_type_id: Required (belongs_to) + + **Relationships:** + - N:1 with members - the member this cycle belongs to + - N:1 with membership_fee_types - the fee type for this cycle + + **Deletion Behavior:** + - ON DELETE CASCADE (member_id): Cycles deleted when member deleted + - ON DELETE RESTRICT (membership_fee_type_id): Cannot delete fee type if cycles exist + ''' +} + // ============================================ // RELATIONSHIPS // ============================================ @@ -306,6 +404,22 @@ Ref: custom_field_values.member_id > members.id [delete: cascade] // - ON DELETE RESTRICT: Cannot delete type if custom_field_values exist Ref: custom_field_values.custom_field_id > custom_fields.id [delete: restrict] +// Member → MembershipFeeType (N:1) +// - Many members can be assigned to one fee type +// - Optional relationship (member can have no fee type) +// - ON DELETE RESTRICT: Cannot delete fee type if members are assigned +Ref: members.membership_fee_type_id > membership_fee_types.id [delete: restrict] + +// MembershipFeeCycle → Member (N:1) +// - Many cycles belong to one member +// - ON DELETE CASCADE: Cycles deleted when member deleted +Ref: membership_fee_cycles.member_id > members.id [delete: cascade] + +// MembershipFeeCycle → MembershipFeeType (N:1) +// - Many cycles reference one fee type +// - ON DELETE RESTRICT: Cannot delete fee type if cycles reference it +Ref: membership_fee_cycles.membership_fee_type_id > membership_fee_types.id [delete: restrict] + // ============================================ // ENUMS // ============================================ @@ -328,6 +442,21 @@ Enum token_purpose { email_confirmation [note: 'Email verification tokens'] } +// Billing interval for membership fee types +Enum membership_fee_interval { + monthly [note: '1st to last day of month'] + quarterly [note: '1st of Jan/Apr/Jul/Oct to last day of quarter'] + half_yearly [note: '1st of Jan/Jul to last day of half'] + yearly [note: 'Jan 1 to Dec 31'] +} + +// Payment status for membership fee cycles +Enum membership_fee_status { + unpaid [note: 'Payment pending (default)'] + paid [note: 'Payment received'] + suspended [note: 'Payment suspended'] +} + // ============================================ // TABLE GROUPS // ============================================ @@ -357,3 +486,17 @@ TableGroup membership_domain { ''' } +TableGroup membership_fees_domain { + membership_fee_types + membership_fee_cycles + + Note: ''' + **Membership Fees Domain** + + Handles membership fee management including: + - Fee type definitions with intervals + - Individual billing cycles per member + - Payment status tracking + ''' +} + diff --git a/docs/feature-roadmap.md b/docs/feature-roadmap.md index 2f86f5e..c4ecfc9 100644 --- a/docs/feature-roadmap.md +++ b/docs/feature-roadmap.md @@ -187,16 +187,16 @@ **Current State:** - ✅ Basic "paid" boolean field on members -- ✅ **UI Mock-ups for Contribution Types & Settings** (2025-12-02) +- ✅ **UI Mock-ups for Membership Fee Types & Settings** (2025-12-02) - ⚠️ No payment tracking **Open Issues:** - [#156](https://git.local-it.org/local-it/mitgliederverwaltung/issues/156) - Set up & document testing environment for vereinfacht.digital (L, Low priority) -- [#226](https://git.local-it.org/local-it/mitgliederverwaltung/issues/226) - Payment/Contribution Mockup Pages (Preview) +- [#226](https://git.local-it.org/local-it/mitgliederverwaltung/issues/226) - Payment/Membership Fee Mockup Pages (Preview) **Mock-Up Pages (Non-Functional Preview):** -- `/contribution_types` - Contribution Types Management -- `/contribution_settings` - Global Contribution Settings +- `/membership_fee_types` - Membership Fee Types Management +- `/membership_fee_settings` - Global Membership Fee Settings **Missing Features:** - ❌ Membership fee configuration diff --git a/docs/membership-fee-architecture.md b/docs/membership-fee-architecture.md new file mode 100644 index 0000000..7c8da24 --- /dev/null +++ b/docs/membership-fee-architecture.md @@ -0,0 +1,723 @@ +# Membership Fees - Technical Architecture + +**Project:** Mila - Membership Management System +**Feature:** Membership Fee Management +**Version:** 1.0 +**Last Updated:** 2025-11-27 +**Status:** Architecture Design - Ready for Implementation + +--- + +## Purpose + +This document defines the technical architecture for the Membership Fees system. It focuses on architectural decisions, patterns, module structure, and integration points **without** concrete implementation details. + +**Related Documents:** + +- [membership-fee-overview.md](./membership-fee-overview.md) - Business logic and requirements +- [database-schema-readme.md](./database-schema-readme.md) - Database documentation +- [database_schema.dbml](./database_schema.dbml) - Database schema definition + +--- + +## Table of Contents + +1. [Architecture Principles](#architecture-principles) +2. [Domain Structure](#domain-structure) +3. [Data Architecture](#data-architecture) +4. [Business Logic Architecture](#business-logic-architecture) +5. [Integration Points](#integration-points) +6. [Acceptance Criteria](#acceptance-criteria) +7. [Testing Strategy](#testing-strategy) +8. [Security Considerations](#security-considerations) +9. [Performance Considerations](#performance-considerations) + +--- + +## Architecture Principles + +### Core Design Decisions + +1. **Single Responsibility:** + - Each module has one clear responsibility + - Cycle generation separated from status management + - Calendar logic isolated in dedicated module + +2. **No Redundancy:** + - No `cycle_end` field (calculated from `cycle_start` + `interval`) + - No `interval_type` field (read from `membership_fee_type.interval`) + - Eliminates data inconsistencies + +3. **Immutability Where Important:** + - `membership_fee_type.interval` cannot be changed after creation + - Prevents complex migration scenarios + - Enforced via Ash change validation + +4. **Historical Accuracy:** + - `amount` stored per cycle for audit trail + - Enables tracking of membership fee changes over time + - Old cycles retain original amounts + +5. **Calendar-Based Cycles:** + - All cycles aligned to calendar boundaries + - Simplifies date calculations + - Predictable cycle generation + +--- + +## Domain Structure + +### Ash Domain: `Mv.MembershipFees` + +**Purpose:** Encapsulates all membership fee-related resources and logic + +**Resources:** + +- `MembershipFeeType` - Membership fee type definitions (admin-managed) +- `MembershipFeeCycle` - Individual membership fee cycles per member + +**Extensions:** + +- Member resource extended with membership fee fields + +### Module Organization + +``` +lib/ +├── membership_fees/ +│ ├── membership_fees.ex # Ash domain definition +│ ├── membership_fee_type.ex # MembershipFeeType resource +│ ├── membership_fee_cycle.ex # MembershipFeeCycle resource +│ └── changes/ +│ ├── prevent_interval_change.ex # Validates interval immutability +│ ├── set_membership_fee_start_date.ex # Auto-sets start date +│ └── validate_same_interval.ex # Validates interval match on type change +├── mv/ +│ └── membership_fees/ +│ ├── cycle_generator.ex # Cycle generation algorithm +│ └── calendar_cycles.ex # Calendar cycle calculations +└── membership/ + └── member.ex # Extended with membership fee relationships +``` + +### Separation of Concerns + +**Domain Layer (Ash Resources):** + +- Data validation +- Relationship management +- Policy enforcement +- Action definitions + +**Business Logic Layer (`Mv.MembershipFees`):** + +- Cycle generation algorithm +- Calendar calculations +- Date boundary handling +- Status transitions + +**UI Layer (LiveView):** + +- User interaction +- Display logic +- Authorization checks +- Form handling + +--- + +## Data Architecture + +### Database Schema Extensions + +**See:** [database-schema-readme.md](./database-schema-readme.md) and [database_schema.dbml](./database_schema.dbml) for complete schema documentation. + +### New Tables + +1. **`membership_fee_types`** + - Purpose: Define membership fee types with fixed intervals + - Key Constraint: `interval` field immutable after creation + - Relationships: has_many members, has_many membership_fee_cycles + +2. **`membership_fee_cycles`** + - Purpose: Individual membership fee cycles for members + - Key Design: NO `cycle_end` or `interval_type` fields (calculated) + - Relationships: belongs_to member, belongs_to membership_fee_type + - Composite uniqueness: One cycle per member per cycle_start + +### Member Table Extensions + +**Fields Added:** + +- `membership_fee_type_id` (FK, NOT NULL with default from settings) +- `membership_fee_start_date` (Date, nullable) + +**Existing Fields Used:** + +- `join_date` - For calculating membership fee start +- `exit_date` - For limiting cycle generation +- These fields must remain member fields and should not be replaced by custom fields in the future + +### Settings Integration + +**Global Settings:** + +- `membership_fees.include_joining_cycle` (Boolean) +- `membership_fees.default_membership_fee_type_id` (UUID) + +**Storage:** Existing settings mechanism (TBD: dedicated table or configuration resource) + +### Foreign Key Behaviors + +| Relationship | On Delete | Rationale | +|--------------|-----------|-----------| +| `membership_fee_cycles.member_id → members.id` | CASCADE | Remove membership fee cycles when member deleted | +| `membership_fee_cycles.membership_fee_type_id → membership_fee_types.id` | RESTRICT | Prevent membership fee type deletion if cycles exist | +| `members.membership_fee_type_id → membership_fee_types.id` | RESTRICT | Prevent membership fee type deletion if assigned to members | + +--- + +## Business Logic Architecture + +### Cycle Generation System + +**Component:** `Mv.MembershipFees.CycleGenerator` + +**Responsibilities:** + +- Calculate which cycles should exist for a member +- Generate missing cycles +- Respect membership_fee_start_date and exit_date boundaries +- Skip existing cycles (idempotent) +- Use PostgreSQL advisory locks per member to prevent race conditions + +**Triggers:** + +1. Member membership fee type assigned (via Ash change) +2. Member created with membership fee type (via Ash change) +3. Scheduled job runs (daily/weekly cron) +4. Admin manual regeneration (UI action) + +**Algorithm Steps:** + +1. Retrieve member with membership fee type and dates +2. Determine generation start point: + - If NO cycles exist: Start from `membership_fee_start_date` (or calculated from `join_date`) + - If cycles exist: Start from the cycle AFTER the last existing one +3. Generate all cycle starts from the determined start point to today (or `exit_date`) +4. Create new cycles with current membership fee type's amount +5. Use PostgreSQL advisory locks per member to prevent race conditions + +**Edge Case Handling:** + +- If membership_fee_start_date is NULL: Calculate from join_date + global setting +- If exit_date is set: Stop generation at exit_date +- If membership fee type changes: Handled separately by regeneration logic +- **Gap Handling:** If cycles were explicitly deleted (gaps exist), they are NOT recreated. + The generator always continues from the cycle AFTER the last existing cycle, regardless of gaps. + +### Calendar Cycle Calculations + +**Component:** `Mv.MembershipFees.CalendarCycles` + +**Responsibilities:** + +- Calculate cycle boundaries based on interval type +- Determine current cycle +- Determine last completed cycle +- Calculate cycle_end from cycle_start + interval + +**Functions (high-level):** + +- `calculate_cycle_start/3` - Given date and interval, find cycle start +- `calculate_cycle_end/2` - Given cycle_start and interval, calculate end +- `next_cycle_start/2` - Given cycle_start and interval, find next +- `is_current_cycle?/2` - Check if cycle contains today +- `is_last_completed_cycle?/2` - Check if cycle just ended + +**Interval Logic:** + +- **Monthly:** Start = 1st of month, End = last day of month +- **Quarterly:** Start = 1st of quarter (Jan/Apr/Jul/Oct), End = last day of quarter +- **Half-yearly:** Start = 1st of half (Jan/Jul), End = last day of half +- **Yearly:** Start = Jan 1st, End = Dec 31st + +### Status Management + +**Component:** Ash actions on `MembershipFeeCycle` + +**Status Transitions:** + +- Simple state machine: unpaid ↔ paid ↔ suspended +- No complex validation (all transitions allowed) +- Permissions checked via Ash policies + +**Actions Required:** + +- `mark_as_paid` - Set status to :paid +- `mark_as_suspended` - Set status to :suspended +- `mark_as_unpaid` - Set status to :unpaid (error correction) + +**Bulk Operations:** + +- `bulk_mark_as_paid` - Mark multiple cycles as paid (efficiency) + - low priority, can be a future issue + +### Membership Fee Type Change Handling + +**Component:** Ash change on `Member.membership_fee_type_id` + +**Validation:** + +- Check if new type has same interval as old type +- If different: Reject change (MVP constraint) +- If same: Allow change + +**Side Effects on Allowed Change:** + +1. Keep all existing cycles unchanged +2. Find future unpaid cycles +3. Delete future unpaid cycles +4. Regenerate cycles with new membership_fee_type_id and amount + +**Implementation Pattern:** + +- Use Ash change module to validate +- Use after_action hook to trigger regeneration synchronously +- Regeneration runs in the same transaction as the member update to ensure atomicity +- CycleGenerator uses advisory locks and transactions internally to prevent race conditions + +**Validation Behavior:** + +- Fail-closed: If membership fee types cannot be loaded during validation, the change is rejected with a validation error +- Nil assignment prevention: Attempts to set membership_fee_type_id to nil are rejected when a current type exists + +--- + +## Integration Points + +### Member Resource Integration + +**Extension Points:** + +1. Add fields via migration +2. Add relationships (belongs_to, has_many) +3. Add calculations (current_cycle_status, overdue_count) +4. Add changes (auto-set membership_fee_start_date, validate interval) + +**Backward Compatibility:** + +- New fields nullable or with defaults +- Existing members get default membership fee type from settings +- No breaking changes to existing member functionality + +### Settings System Integration + +**Requirements:** + +- Store two global settings +- Provide UI for admin to modify +- Default values if not set +- Validation (e.g., default membership fee type must exist) + +**Access Pattern:** + +- Read settings during cycle generation +- Read settings during member creation +- Write settings only via admin UI + +### Permission System Integration + +**See:** [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) + +**Required Permissions:** + +- `MembershipFeeType.create/update/destroy` - Admin only +- `MembershipFeeType.read` - Admin, Treasurer, Board +- `MembershipFeeCycle.update` (status changes) - Admin, Treasurer +- `MembershipFeeCycle.read` - Admin, Treasurer, Board, Own member + +**Policy Patterns:** + +- Use existing HasPermission check +- Leverage existing roles (Admin, Kassenwart) +- Member can read own cycles (linked via member_id) + +### LiveView Integration + +**New LiveViews Required:** + +1. MembershipFeeType index/form (admin) +2. MembershipFeeCycle table component (member detail view) +3. Settings form section (admin) +4. Member list column (membership fee status) + +**Existing LiveViews to Extend:** + +- Member detail view: Add membership fees section +- Member list view: Add status column +- Settings page: Add membership fees section + +**Authorization Helpers:** + +- Use existing `can?/3` helper for UI conditionals +- Check permissions before showing actions + +--- + +## Acceptance Criteria + +### MembershipFeeType Resource + +**AC-MFT-1:** Admin can create membership fee type with name, amount, interval, description +**AC-MFT-2:** Interval field is immutable after creation (validation error on change attempt) +**AC-MFT-3:** Admin can update name, amount, description (but not interval) +**AC-MFT-4:** Cannot delete membership fee type if assigned to members +**AC-MFT-5:** Cannot delete membership fee type if cycles exist referencing it +**AC-MFT-6:** Interval must be one of: monthly, quarterly, half_yearly, yearly + +### MembershipFeeCycle Resource + +**AC-MFC-1:** Cycle has cycle_start, status, amount, notes, member_id, membership_fee_type_id +**AC-MFC-2:** cycle_end is calculated, not stored +**AC-MFC-3:** Status defaults to :unpaid +**AC-MFC-4:** One cycle per member per cycle_start (uniqueness constraint) +**AC-MFC-5:** Amount is set at generation time from membership_fee_type.amount +**AC-MFC-6:** Cycles cascade delete when member deleted +**AC-MFC-7:** Admin/Treasurer can change status +**AC-MFC-8:** Member can read own cycles + +### Member Extensions + +**AC-M-1:** Member has membership_fee_type_id field (NOT NULL with default) +**AC-M-2:** Member has membership_fee_start_date field (nullable) +**AC-M-3:** New members get default membership fee type from global setting +**AC-M-4:** membership_fee_start_date auto-set based on join_date and global setting +**AC-M-5:** Admin can manually override membership_fee_start_date +**AC-M-6:** Cannot change to membership fee type with different interval (MVP) + +### Cycle Generation + +**AC-CG-1:** Cycles generated when member gets membership fee type +**AC-CG-2:** Cycles generated when member created (via change hook) +**AC-CG-3:** Scheduled job generates missing cycles daily +**AC-CG-4:** Generation respects membership_fee_start_date +**AC-CG-5:** Generation stops at exit_date if member exited +**AC-CG-6:** Generation is idempotent (skips existing cycles) +**AC-CG-7:** Cycles align to calendar boundaries (1st of month/quarter/half/year) +**AC-CG-8:** Amount comes from membership_fee_type at generation time + +### Calendar Logic + +**AC-CL-1:** Monthly cycles: 1st to last day of month +**AC-CL-2:** Quarterly cycles: 1st of Jan/Apr/Jul/Oct to last day of quarter +**AC-CL-3:** Half-yearly cycles: 1st of Jan/Jul to last day of half +**AC-CL-4:** Yearly cycles: Jan 1 to Dec 31 +**AC-CL-5:** cycle_end calculated correctly for all interval types +**AC-CL-6:** Current cycle determined correctly based on today's date +**AC-CL-7:** Last completed cycle determined correctly + +### Membership Fee Type Change + +**AC-TC-1:** Can change to type with same interval +**AC-TC-2:** Cannot change to type with different interval (error message) +**AC-TC-3:** On allowed change: future unpaid cycles regenerated +**AC-TC-4:** On allowed change: paid/suspended cycles unchanged +**AC-TC-5:** On allowed change: amount updated to new type's amount +**AC-TC-6:** Change is atomic (transaction) - ✅ Implemented: Regeneration runs synchronously in the same transaction as the member update + +### Settings + +**AC-S-1:** Global setting: include_joining_cycle (boolean, default true) +**AC-S-2:** Global setting: default_membership_fee_type_id (UUID, required) +**AC-S-3:** Admin can modify settings via UI +**AC-S-4:** Settings validated (e.g., default membership fee type must exist) +**AC-S-5:** Settings applied to new members immediately + +### UI - Member List + +**AC-UI-ML-1:** New column shows membership fee status +**AC-UI-ML-2:** Default: Shows last completed cycle status +**AC-UI-ML-3:** Optional: Toggle to show current cycle status +**AC-UI-ML-4:** Color coding: green (paid), red (unpaid), gray (suspended) +**AC-UI-ML-5:** Filter: Unpaid in last cycle +**AC-UI-ML-6:** Filter: Unpaid in current cycle + +### UI - Member Detail + +**AC-UI-MD-1:** Membership fees section shows all cycles +**AC-UI-MD-2:** Table columns: Cycle, Interval, Amount, Status, Actions +**AC-UI-MD-3:** Checkbox per cycle for bulk marking (low prio) +**AC-UI-MD-4:** "Mark selected as paid" button +**AC-UI-MD-5:** Dropdown to change membership fee type (same interval only) +**AC-UI-MD-6:** Warning if different interval selected +**AC-UI-MD-7:** Only show actions if user has permission + +### UI - Membership Fee Types Admin + +**AC-UI-CTA-1:** List all membership fee types +**AC-UI-CTA-2:** Show: Name, Amount, Interval, Member count +**AC-UI-CTA-3:** Create new membership fee type form +**AC-UI-CTA-4:** Edit form: Name, Amount, Description editable +**AC-UI-CTA-5:** Edit form: Interval grayed out (not editable) +**AC-UI-CTA-6:** Warning on amount change (explain impact) +**AC-UI-CTA-7:** Cannot delete if members assigned +**AC-UI-CTA-8:** Only admin can access + +### UI - Settings Admin + +**AC-UI-SA-1:** Membership fees section in settings +**AC-UI-SA-2:** Dropdown to select default membership fee type +**AC-UI-SA-3:** Checkbox: Include joining cycle +**AC-UI-SA-4:** Explanatory text with examples +**AC-UI-SA-5:** Save button with validation + +--- + +## Testing Strategy + +### Unit Testing + +**Cycle Generator Tests:** + +- Correct cycle_start calculation for all interval types +- Correct cycle count from start to end date +- Respects membership_fee_start_date boundary +- Respects exit_date boundary +- Skips existing cycles (idempotent) +- Does not fill gaps when cycles were deleted +- Handles edge dates (year boundaries, leap years) + +**Calendar Cycles Tests:** + +- Cycle boundaries correct for all intervals +- cycle_end calculation correct +- Current cycle detection +- Last completed cycle detection +- Next cycle calculation + +**Validation Tests:** + +- Interval immutability enforced +- Same interval validation on type change +- Status transitions allowed +- Uniqueness constraints enforced + +### Integration Testing + +**Cycle Generation Flow:** + +- Member creation triggers generation +- Type assignment triggers generation +- Type change regenerates future cycles +- Scheduled job generates missing cycles +- Left member stops generation + +**Status Management Flow:** + +- Mark single cycle as paid +- Bulk mark multiple cycles (low prio) +- Status transitions work +- Permissions enforced + +**Membership Fee Type Management:** + +- Create type +- Update amount (regeneration triggered) +- Cannot update interval +- Cannot delete if in use + +### LiveView Testing + +**Member List:** + +- Status column displays correctly +- Toggle between last/current works +- Filters work correctly +- Color coding applied + +**Member Detail:** + +- Cycles table displays all cycles +- Checkboxes work +- Bulk marking works (low prio) +- Membership fee type change validation works +- Actions only shown with permission + +**Admin UI:** + +- Type CRUD works +- Settings save correctly +- Validations display errors +- Only authorized users can access + +### Edge Case Testing + +**Interval Change Attempt:** + +- Error message displayed +- No data modified +- User can cancel/choose different type + +**Exit with Unpaid:** + +- Warning shown +- Option to suspend offered +- Exit completes correctly + +**Amount Change:** + +- Warning displayed +- Only future unpaid regenerated +- Historical cycles unchanged + +**Date Boundaries:** + +- Today = cycle start handled +- Today = cycle end handled +- Leap year handled + +### Performance Testing + +**Cycle Generation:** + +- Generate 10 years of monthly cycles: < 100ms +- Generate for 1000 members: < 5 seconds +- Idempotent check efficient (no full scan) + +**Member List Query:** + +- With status column: < 200ms for 1000 members +- Filters applied efficiently +- No N+1 queries + +--- + +## Security Considerations + +### Authorization + +**Permissions Required:** + +- Membership fee type management: Admin only +- Membership fee cycle status changes: Admin + Treasurer +- View all cycles: Admin + Treasurer + Board +- View own cycles: All authenticated users + +**Policy Enforcement:** + +- All actions protected by Ash policies +- UI shows/hides based on permissions +- Backend validates permissions (never trust UI alone) + +### Data Integrity + +**Validation Layers:** + +1. Database constraints (NOT NULL, UNIQUE, CHECK) +2. Ash validations (business rules) +3. UI validations (user experience) + +**Immutability Protection:** + +- Interval change prevented at multiple layers +- Cycle amounts immutable (audit trail) +- Settings changes logged (future) + +### Audit Trail + +**Tracked Information:** + +- Cycle status changes (who, when) - future enhancement +- Membership fee type amount changes (implicit via cycle amounts) + +--- + +## Performance Considerations + +### Database Indexes + +**Required Indexes:** + +- `membership_fee_cycles(member_id)` - For member cycle lookups +- `membership_fee_cycles(membership_fee_type_id)` - For type queries +- `membership_fee_cycles(status)` - For unpaid filters +- `membership_fee_cycles(cycle_start)` - For date range queries +- `membership_fee_cycles(member_id, cycle_start)` - Composite unique index +- `members(membership_fee_type_id)` - For type membership count + +### Query Optimization + +**Preloading:** + +- Load membership_fee_type with cycles (avoid N+1) +- Load cycles when displaying member detail +- Use Ash's load for efficient preloading + +**Calculated Fields:** + +- cycle_end calculated on-demand (not stored) +- current_cycle_status calculated when needed +- Use Ash calculations for lazy evaluation + +**Pagination:** + +- Cycle list paginated if > 50 cycles +- Member list already paginated + +### Caching Strategy + +**No caching needed in MVP:** + +- Membership fee types rarely change +- Cycle queries are fast +- Settings read infrequently + +**Future caching if needed:** + +- Cache settings in application memory +- Cache membership fee types list +- Invalidate on change + +### Scheduled Job Performance + +**Cycle Generation Job:** + +- Run daily or weekly (not hourly) +- Batch members (process 100 at a time) +- Skip members with no changes +- Log failures for retry + +--- + +## Future Enhancements + +### Phase 2: Interval Change Support + +**Architecture Changes:** + +- Add logic to handle cycle overlaps +- Calculate prorata amounts if needed +- More complex validation +- Migration path for existing cycles + +### Phase 3: Payment Details + +**Architecture Changes:** + +- Add PaymentTransaction resource +- Link transactions to cycles +- Support multiple payments per cycle +- Reconciliation logic + +### Phase 4: vereinfacht.digital Integration + +**Architecture Changes:** + +- External API client module +- Webhook handling for transactions +- Automatic matching logic +- Manual review interface + +--- + +**End of Architecture Document** diff --git a/docs/contributions-overview.md b/docs/membership-fee-overview.md similarity index 58% rename from docs/contributions-overview.md rename to docs/membership-fee-overview.md index e0c4bc8..bd47faa 100644 --- a/docs/contributions-overview.md +++ b/docs/membership-fee-overview.md @@ -1,7 +1,7 @@ -# Membership Contributions - Overview +# Membership Fees - Overview **Project:** Mila - Membership Management System -**Feature:** Membership Contribution Management +**Feature:** Membership Fee Management **Version:** 1.0 **Last Updated:** 2025-11-27 **Status:** Concept - Ready for Review @@ -10,9 +10,9 @@ ## Purpose -This document provides a comprehensive overview of the Membership Contributions system. It covers business logic, data model, UI/UX design, and technical architecture in a concise, bullet-point format. +This document provides a comprehensive overview of the Membership Fees system. It covers business logic, data model, UI/UX design, and technical architecture in a concise, bullet-point format. -**For detailed implementation:** See [contributions-implementation-plan.md](./contributions-implementation-plan.md) (created after concept iterations) +**For detailed implementation:** See [membership-fee-implementation-plan.md](./membership-fee-implementation-plan.md) (created after concept iterations) --- @@ -36,7 +36,7 @@ This document provides a comprehensive overview of the Membership Contributions - Minimal complexity - Clear data model without redundancies - Intuitive operation -- Calendar period-based (Month/Quarter/Half-Year/Year) +- Calendar cycle-based (Month/Quarter/Half-Year/Year) --- @@ -46,9 +46,9 @@ This document provides a comprehensive overview of the Membership Contributions **Core Entities:** -- Beitragsart ↔ Contribution Type / Membership Fee Type -- Beitragsintervall ↔ Contribution Period -- Mitgliedsbeitrag ↔ Membership Fee / Contribution +- Beitragsart ↔ Membership Fee Type +- Beitragszyklus ↔ Membership Fee Cycle +- Mitgliedsbeitrag ↔ Membership Fee **Status:** @@ -56,7 +56,7 @@ This document provides a comprehensive overview of the Membership Contributions - unbezahlt ↔ unpaid - ausgesetzt ↔ suspended / waived -**Intervals:** +**Intervals (Frequenz / Payment Frequency):** - monatlich ↔ monthly - quartalsweise ↔ quarterly @@ -65,8 +65,8 @@ This document provides a comprehensive overview of the Membership Contributions **UI Elements:** -- "Letztes Intervall" ↔ "Last Period" (e.g., 2023 when in 2024) -- "Aktuelles Intervall" ↔ "Current Period" (e.g., 2024) +- "Letzter Zyklus" ↔ "Last Cycle" (e.g., 2023 when in 2024) +- "Aktueller Zyklus" ↔ "Current Cycle" (e.g., 2024) - "Als bezahlt markieren" ↔ "Mark as paid" - "Aussetzen" ↔ "Suspend" / "Waive" @@ -74,43 +74,41 @@ This document provides a comprehensive overview of the Membership Contributions ## Data Model -### Contribution Type (ContributionType) +### Membership Fee Type (MembershipFeeType) ``` - id (UUID) - name (String) - e.g., "Regular", "Reduced", "Student" -- amount (Decimal) - Contribution amount in Euro +- amount (Decimal) - Membership fee amount in Euro - interval (Enum) - :monthly, :quarterly, :half_yearly, :yearly - description (Text, optional) -- timestamps ``` **Important:** - `interval` is **IMMUTABLE** after creation! - Admin can only change `name`, `amount`, `description` -- On change: Future unpaid periods regenerated with new amount +- On change: Future unpaid cycles regenerated with new amount -### Contribution Period (ContributionPeriod) +### Membership Fee Cycle (MembershipFeeCycle) ``` - id (UUID) - member_id (FK → members.id) -- contribution_type_id (FK → contribution_types.id) -- period_start (Date) - Calendar period start (01.01., 01.04., 01.07., 01.10., etc.) +- membership_fee_type_id (FK → membership_fee_types.id) +- cycle_start (Date) - Calendar cycle start (01.01., 01.04., 01.07., 01.10., etc.) - status (Enum) - :unpaid (default), :paid, :suspended -- amount (Decimal) - Amount at generation time (history when type changes) +- amount (Decimal) - Membership fee amount at generation time (history when type changes) - notes (Text, optional) - Admin notes -- timestamps ``` **Important:** -- **NO** `period_end` - calculated from `period_start` + `interval` -- **NO** `interval_type` - read from `contribution_type.interval` +- **NO** `cycle_end` - calculated from `cycle_start` + `interval` +- **NO** `interval_type` - read from `membership_fee_type.interval` - Avoids redundancy and inconsistencies! -**Calendar Period Logic:** +**Calendar Cycle Logic:** - Monthly: 01.01. - 31.01., 01.02. - 28./29.02., etc. - Quarterly: 01.01. - 31.03., 01.04. - 30.06., 01.07. - 30.09., 01.10. - 31.12. @@ -120,70 +118,76 @@ This document provides a comprehensive overview of the Membership Contributions ### Member (Extensions) ``` -- contribution_type_id (FK → contribution_types.id, NOT NULL, default from settings) -- contribution_start_date (Date, nullable) - When to start generating contributions -- left_at (Date, nullable) - Exit date (existing) +- membership_fee_type_id (FK → membership_fee_types.id, NOT NULL, default from settings) +- membership_fee_start_date (Date, nullable) - When to start generating membership fees +- exit_date (Date, nullable) - Exit date (existing) ``` -**Logic for contribution_start_date:** +**Logic for membership_fee_start_date:** -- Auto-set based on global setting `include_joining_period` -- If `include_joining_period = true`: First day of joining month/quarter/year -- If `include_joining_period = false`: First day of NEXT period after joining +- Auto-set based on global setting `include_joining_cycle` +- If `include_joining_cycle = true`: First day of joining month/quarter/year +- If `include_joining_cycle = false`: First day of NEXT cycle after joining - Can be manually overridden by admin -**NO** `include_joining_period` field on Member - unnecessary due to `contribution_start_date`! +**NO** `include_joining_cycle` field on Member - unnecessary due to `membership_fee_start_date`! ### Global Settings ``` -key: "contributions.include_joining_period" +key: "membership_fees.include_joining_cycle" value: Boolean (Default: true) -key: "contributions.default_contribution_type_id" -value: UUID (Required) - Default contribution type for new members +key: "membership_fees.default_membership_fee_type_id" +value: UUID (Required) - Default membership fee type for new members ``` -**Meaning include_joining_period:** +**Meaning include_joining_cycle:** -- `true`: Joining period is included (member pays from joining period) -- `false`: Only from next full period after joining +- `true`: Joining cycle is included (member pays from joining cycle) +- `false`: Only from next full cycle after joining -**Meaning default_contribution_type_id:** +**Meaning of default membership fee type setting:** -- Every new member automatically gets this contribution type +- Every new member automatically gets this membership fee type - Must be configured in admin settings -- Prevents: Members without contribution type +- Prevents: Members without membership fee type --- ## Business Logic -### Period Generation +### Cycle Generation **Triggers:** -- Member gets contribution type assigned (also during member creation) -- New period begins (Cron job daily/weekly) +- Member gets membership fee type assigned (also during member creation) +- New cycle begins (Cron job daily/weekly) - Admin requests manual regeneration **Algorithm:** -1. Get `member.contribution_start_date` and `member.contribution_type` -2. Calculate first period based on `contribution_start_date` -3. Generate all periods from start to today (or `left_at` if present) -4. Skip existing periods -5. Set `amount` to current `contribution_type.amount` +Use PostgreSQL advisory locks per member to prevent race conditions + +1. Get `member.membership_fee_start_date` and member's membership fee type +2. Determine generation start point: + - If NO cycles exist: Start from `membership_fee_start_date` + - If cycles exist: Start from the cycle AFTER the last existing one +3. Generate cycles until today (or `exit_date` if present): + - Use the interval to generate the cycles + - **Note:** If cycles were explicitly deleted (gaps exist), they are NOT recreated. + The generator always continues from the cycle AFTER the last existing cycle. +4. Set `amount` to current membership fee type's amount **Example (Yearly):** ``` Joining date: 15.03.2023 -include_joining_period: true -→ contribution_start_date: 01.01.2023 +include_joining_cycle: true +→ membership_fee_start_date: 01.01.2023 -Generated periods: -- 01.01.2023 - 31.12.2023 (joining period) +Generated cycles: +- 01.01.2023 - 31.12.2023 (joining cycle) - 01.01.2024 - 31.12.2024 - 01.01.2025 - 31.12.2025 (current year) ``` @@ -192,10 +196,10 @@ Generated periods: ``` Joining date: 15.03.2023 -include_joining_period: false -→ contribution_start_date: 01.04.2023 +include_joining_cycle: false +→ membership_fee_start_date: 01.04.2023 -Generated periods: +Generated cycles: - 01.04.2023 - 30.06.2023 (first full quarter) - 01.07.2023 - 30.09.2023 - 01.10.2023 - 31.12.2023 @@ -218,44 +222,44 @@ suspended → unpaid - Admin + Treasurer (Kassenwart) can change status - Uses existing permission system -### Contribution Type Change +### Membership Fee Type Change -**MVP - Same Interval Only:** +**MVP - Same Cycle Only:** -- Member can only choose contribution type with **same interval** +- Member can only choose membership fee type with **same cycle** - Example: From "Regular (yearly)" to "Reduced (yearly)" ✓ - Example: From "Regular (yearly)" to "Reduced (monthly)" ✗ **Logic on Change:** -1. Check: New contribution type has same interval -2. If yes: Set `member.contribution_type_id` -3. Future **unpaid** periods: Delete and regenerate with new amount -4. Paid/suspended periods: Remain unchanged (historical amount) +1. Check: New membership fee type has same interval +2. If yes: Set `member.membership_fee_type_id` +3. Future **unpaid** cycles: Delete and regenerate with new amount +4. Paid/suspended cycles: Remain unchanged (historical amount) **Future - Different Intervals:** - Enable interval switching (e.g., yearly → monthly) -- More complex logic for period overlaps +- More complex logic for cycle overlaps - Needs additional validation ### Member Exit **Logic:** -- Periods only generated until `member.left_at` -- Existing periods remain visible -- Unpaid exit period can be marked as "suspended" +- Cycles only generated until `member.exit_date` +- Existing cycles remain visible +- Unpaid exit cycle can be marked as "suspended" **Example:** ``` Exit: 15.08.2024 -Yearly period: 01.01.2024 - 31.12.2024 +Yearly cycle: 01.01.2024 - 31.12.2024 -→ Period 2024 is shown (Status: unpaid) +→ Cycle 2024 is shown (Status: unpaid) → Admin can set to "suspended" -→ No periods for 2025+ generated +→ No cycles for 2025+ generated ``` --- @@ -264,46 +268,46 @@ Yearly period: 01.01.2024 - 31.12.2024 ### Member List View -**New Column: "Contribution Status"** +**New Column: "Membership Fee Status"** -**Default Display (Last Period):** +**Default Display (Last Cycle):** -- Shows status of **last completed** period -- Example in 2024: Shows contribution for 2023 +- Shows status of **last completed** cycle +- Example in 2024: Shows membership fee for 2023 - Color coding: - Green: paid ✓ - Red: unpaid ✗ - Gray: suspended ⊘ -**Optional: Show Current Period** +**Optional: Show Current Cycle** -- Toggle: "Show current period" (2024) +- Toggle: "Show current cycle" (2024) - Admin decides what to display **Filters:** -- "Unpaid contributions in last period" -- "Unpaid contributions in current period" +- "Unpaid membership fees in last cycle" +- "Unpaid membership fees in current cycle" ### Member Detail View -**Section: "Contributions"** +**Section: "Membership Fees"** -**Contribution Type Assignment:** +**Membership Fee Type Assignment:** ``` ┌─────────────────────────────────────┐ -│ Contribution Type: [Dropdown] │ -│ ⚠ Only types with same interval │ -│ can be selected │ +│ Membership Fee Type: [Dropdown] │ +│ ⚠ Only types with same interval │ +│ can be selected │ └─────────────────────────────────────┘ ``` -**Period Table:** +**Cycle Table:** ``` ┌───────────────┬──────────┬────────┬──────────┬─────────┐ -│ Period │ Interval │ Amount │ Status │ Action │ +│ Cycle │ Interval │ Amount │ Status │ Action │ ├───────────────┼──────────┼────────┼──────────┼─────────┤ │ 01.01.2023- │ Yearly │ 50 € │ ☑ Paid │ │ │ 31.12.2023 │ │ │ │ │ @@ -322,9 +326,9 @@ Legend: ☑ = paid | ☐ = unpaid | ⊘ = suspended - Checkbox in each row for fast marking - Button: "Mark selected as paid/unpaid/suspended" -- Bulk action for multiple periods +- Bulk action for multiple cycles -### Admin: Contribution Types Management +### Admin: Membership Fee Types Management **List:** @@ -352,37 +356,37 @@ Legend: ☑ = paid | ☐ = unpaid | ⊘ = suspended Impact: - 45 members affected -- Future unpaid periods will be generated with 65 € -- Already paid periods remain with old amount +- Future unpaid cycles will be generated with 65 € +- Already paid cycles remain with old amount [Cancel] [Confirm] ``` ### Admin: Settings -**Contribution Configuration:** +**Membership Fee Configuration:** ``` -Default Contribution Type: [Dropdown: Contribution Types] +Default Membership Fee Type: [Dropdown: Membership Fee Types] Selected: "Regular (60 €, Yearly)" -This contribution type is automatically assigned to all new members. +This membership fee type is automatically assigned to all new members. Can be changed individually per member. --- -☐ Include joining period +☐ Include joining cycle When active: -Members pay from the period of their joining. +Members pay from the cycle of their joining. Example (Yearly): Joining: 15.03.2023 → Pays from 2023 When inactive: -Members pay from the next full period. +Members pay from the next full cycle. Example (Yearly): Joining: 15.03.2023 @@ -393,7 +397,7 @@ Joining: 15.03.2023 ## Edge Cases -### 1. Contribution Type Change with Different Interval +### 1. Membership Fee Type Change with Different Interval **MVP:** Blocked (only same interval allowed) @@ -402,11 +406,11 @@ Joining: 15.03.2023 ``` Error: Interval change not possible -Current contribution type: "Regular (Yearly)" -Selected contribution type: "Student (Monthly)" +Current membership fee type: "Regular (Yearly)" +Selected membership fee type: "Student (Monthly)" Changing the interval is currently not possible. -Please select a contribution type with interval "Yearly". +Please select a membership fee type with interval "Yearly". [OK] ``` @@ -415,32 +419,32 @@ Please select a contribution type with interval "Yearly". - Allow interval switching - Calculate overlaps -- Generate new periods without duplicates +- Generate new cycles without duplicates -### 2. Exit with Unpaid Contributions +### 2. Exit with Unpaid Membership Fees **Scenario:** ``` Member exits: 15.08.2024 -Yearly period 2024: unpaid +Yearly cycle 2024: unpaid ``` **UI Notice on Exit: (Low Prio)** ``` -⚠ Unpaid contributions present +⚠ Unpaid membership fees present -This member has 1 unpaid period(s): +This member has 1 unpaid cycle(s): - 2024: 60 € (unpaid) Do you want to continue? -[ ] Mark contribution as "suspended" +[ ] Mark membership fee as "suspended" [Cancel] [Confirm Exit] ``` -### 3. Multiple Unpaid Periods +### 3. Multiple Unpaid Cycles **Scenario:** Member hasn't paid for 2 years @@ -467,9 +471,9 @@ Do you want to continue? **Result:** -- Period 2023: Saved with 50 € (history) -- Period 2024: Generated with 60 € (current) -- Both periods show correct historical amount +- Cycle 2023: Saved with 50 € (history) +- Cycle 2024: Generated with 60 € (current) +- Both cycles show correct historical amount ### 5. Date Boundaries @@ -477,7 +481,7 @@ Do you want to continue? **Solution:** -- Current period (2025) is generated +- Current cycle (2025) is generated - Status: unpaid (open) - Shown in overview @@ -489,17 +493,17 @@ Do you want to continue? **Included:** -- ✓ Contribution types (CRUD) -- ✓ Automatic period generation +- ✓ Membership fee types (CRUD) +- ✓ Automatic cycle generation - ✓ Status management (paid/unpaid/suspended) -- ✓ Member overview with contribution status -- ✓ Period view per member +- ✓ Member overview with membership fee status +- ✓ Cycle view per member - ✓ Quick checkbox marking - ✓ Bulk actions - ✓ Amount history - ✓ Same-interval type change -- ✓ Default contribution type -- ✓ Joining period configuration +- ✓ Default membership fee type +- ✓ Joining cycle configuration **NOT Included:** @@ -515,7 +519,7 @@ Do you want to continue? **Phase 2:** - Payment details (date, amount, method) -- Interval change for future unpaid periods +- Interval change for future unpaid cycles - Manual vereinfacht.digital links per member - Extended filter options diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index fa45d86..b44604b 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -93,8 +93,8 @@ Five predefined roles stored in the `roles` table: Control CRUD operations on: - User (credentials, profile) - Member (member data) -- Property (custom field values) -- PropertyType (custom field definitions) +- CustomFieldValue (custom field values) +- CustomField (custom field definitions) - Role (role management) **4. Page-Level Permissions** @@ -111,7 +111,7 @@ Three scope levels for permissions: - **:own** - Only records where `record.id == user.id` (for User resource) - **:linked** - Only records linked to user via relationships - Member: `member.user_id == user.id` - - Property: `property.member.user_id == user.id` + - CustomFieldValue: `custom_field_value.member.user_id == user.id` - **:all** - All records, no filtering **6. Special Cases** @@ -414,7 +414,7 @@ defmodule Mv.Authorization.PermissionSets do ## Permission Sets 1. **own_data** - Default for "Mitglied" role - - Can only access own user data and linked member/properties + - Can only access own user data and linked member/custom field values - Cannot create new members or manage system 2. **read_only** - For "Vorstand" and "Buchhaltung" roles @@ -423,11 +423,11 @@ defmodule Mv.Authorization.PermissionSets do 3. **normal_user** - For "Kassenwart" role - Create/Read/Update members (no delete), full CRUD on properties - - Cannot manage property types or users + - Cannot manage custom fields or users 4. **admin** - For "Admin" role - Unrestricted access to all resources - - Can manage users, roles, property types + - Can manage users, roles, custom fields ## Usage @@ -500,12 +500,12 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # Property: Can read/update properties of linked member - %{resource: "Property", action: :read, scope: :linked, granted: true}, - %{resource: "Property", action: :update, scope: :linked, granted: true}, + # CustomFieldValue: Can read/update custom field values of linked member + %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, - # PropertyType: Can read all (needed for forms) - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Can read all (needed for forms) + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", # Home page @@ -525,17 +525,17 @@ defmodule Mv.Authorization.PermissionSets do # Member: Can read all members, no modifications %{resource: "Member", action: :read, scope: :all, granted: true}, - # Property: Can read all properties - %{resource: "Property", action: :read, scope: :all, granted: true}, + # CustomFieldValue: Can read all custom field values + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - # PropertyType: Can read all - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Can read all + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", "/members", # Member list "/members/:id", # Member detail - "/properties", # Property overview + "/custom_field_values" # Custom field values overview "/profile" # Own profile ] } @@ -554,14 +554,14 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :update, scope: :all, granted: true}, # Note: destroy intentionally omitted for safety - # Property: Full CRUD - %{resource: "Property", action: :read, scope: :all, granted: true}, - %{resource: "Property", action: :create, scope: :all, granted: true}, - %{resource: "Property", action: :update, scope: :all, granted: true}, - %{resource: "Property", action: :destroy, scope: :all, granted: true}, + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - # PropertyType: Read only (admin manages definitions) - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Read only (admin manages definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", @@ -569,9 +569,9 @@ defmodule Mv.Authorization.PermissionSets do "/members/new", # Create member "/members/:id", "/members/:id/edit", # Edit member - "/properties", - "/properties/new", - "/properties/:id/edit", + "/custom_field_values", + "/custom_field_values/new", + "/custom_field_values/:id/edit", "/profile" ] } @@ -592,17 +592,17 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :update, scope: :all, granted: true}, %{resource: "Member", action: :destroy, scope: :all, granted: true}, - # Property: Full CRUD - %{resource: "Property", action: :read, scope: :all, granted: true}, - %{resource: "Property", action: :create, scope: :all, granted: true}, - %{resource: "Property", action: :update, scope: :all, granted: true}, - %{resource: "Property", action: :destroy, scope: :all, granted: true}, + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - # PropertyType: Full CRUD (admin manages custom field definitions) - %{resource: "PropertyType", action: :read, scope: :all, granted: true}, - %{resource: "PropertyType", action: :create, scope: :all, granted: true}, - %{resource: "PropertyType", action: :update, scope: :all, granted: true}, - %{resource: "PropertyType", action: :destroy, scope: :all, granted: true}, + # CustomField: Full CRUD (admin manages custom field definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true}, + %{resource: "CustomField", action: :create, scope: :all, granted: true}, + %{resource: "CustomField", action: :update, scope: :all, granted: true}, + %{resource: "CustomField", action: :destroy, scope: :all, granted: true}, # Role: Full CRUD (admin manages roles) %{resource: "Role", action: :read, scope: :all, granted: true}, @@ -677,9 +677,9 @@ Quick reference table showing what each permission set allows: | **User** (all) | - | - | - | R, C, U, D | | **Member** (linked) | R, U | - | - | - | | **Member** (all) | - | R | R, C, U | R, C, U, D | -| **Property** (linked) | R, U | - | - | - | -| **Property** (all) | - | R | R, C, U, D | R, C, U, D | -| **PropertyType** (all) | R | R | R | R, C, U, D | +| **CustomFieldValue** (linked) | R, U | - | - | - | +| **CustomFieldValue** (all) | - | R | R, C, U, D | R, C, U, D | +| **CustomField** (all) | R | R | R | R, C, U, D | | **Role** (all) | - | - | - | R, C, U, D | **Legend:** R=Read, C=Create, U=Update, D=Destroy @@ -715,7 +715,7 @@ defmodule Mv.Authorization.Checks.HasPermission do - **:own** - Filters to records where record.id == actor.id - **:linked** - Filters based on resource type: - Member: member.user_id == actor.id - - Property: property.member.user_id == actor.id (traverses relationship!) + - CustomFieldValue: custom_field_value.member.user_id == actor.id (traverses relationship!) ## Error Handling @@ -802,8 +802,8 @@ defmodule Mv.Authorization.Checks.HasPermission do # Member.user_id == actor.id (direct relationship) {:filter, expr(user_id == ^actor.id)} - "Property" -> - # Property.member.user_id == actor.id (traverse through member!) + "CustomFieldValue" -> + # CustomFieldValue.member.user_id == actor.id (traverse through member!) {:filter, expr(member.user_id == ^actor.id)} _ -> @@ -832,7 +832,7 @@ end **Key Design Decisions:** -1. **Resource-Specific :linked Scope:** Property needs to traverse `member` relationship to check `user_id` +1. **Resource-Specific :linked Scope:** CustomFieldValue needs to traverse `member` relationship to check `user_id` 2. **Error Handling:** All errors log for debugging but return generic forbidden to user 3. **Module Name Extraction:** Uses `Module.split() |> List.last()` to match against PermissionSets strings 4. **Pure Function:** No side effects, deterministic, easily testable @@ -966,21 +966,21 @@ end *Email editing has additional validation (see Special Cases) -### Property Resource Policies +### CustomFieldValue Resource Policies -**Location:** `lib/mv/membership/property.ex` +**Location:** `lib/mv/membership/custom_field_value.ex` -**Special Case:** Users can access properties of their linked member. +**Special Case:** Users can access custom field values of their linked member. ```elixir -defmodule Mv.Membership.Property do +defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ... policies do - # SPECIAL CASE: Users can access properties of their linked member + # SPECIAL CASE: Users can access custom field values of their linked member # Note: This traverses the member relationship! policy action_type([:read, :update]) do - description "Users can access properties of their linked member" + description "Users can access custom field values of their linked member" authorize_if expr(member.user_id == ^actor(:id)) end @@ -1010,18 +1010,18 @@ end | Create | ❌ | ❌ | ✅ | ❌ | ✅ | | Destroy | ❌ | ❌ | ✅ | ❌ | ✅ | -### PropertyType Resource Policies +### CustomField Resource Policies -**Location:** `lib/mv/membership/property_type.ex` +**Location:** `lib/mv/membership/custom_field.ex` **No Special Cases:** All users can read, only admin can write. ```elixir -defmodule Mv.Membership.PropertyType do +defmodule Mv.Membership.CustomField do use Ash.Resource, ... policies do - # All authenticated users can read property types (needed for forms) + # All authenticated users can read custom fields (needed for forms) # Write operations are admin-only policy action_type([:read, :create, :update, :destroy]) do description "Check permissions from user's role" @@ -1308,12 +1308,12 @@ end - ❌ Cannot access: `/members`, `/members/new`, `/admin/roles` **Vorstand (read_only):** -- ✅ Can access: `/`, `/members`, `/members/123`, `/properties`, `/profile` +- ✅ Can access: `/`, `/members`, `/members/123`, `/custom_field_values`, `/profile` - ❌ Cannot access: `/members/new`, `/members/123/edit`, `/admin/roles` **Kassenwart (normal_user):** -- ✅ Can access: `/`, `/members`, `/members/new`, `/members/123/edit`, `/properties`, `/profile` -- ❌ Cannot access: `/admin/roles`, `/admin/property_types/new` +- ✅ Can access: `/`, `/members`, `/members/new`, `/members/123/edit`, `/custom_field_values`, `/profile` +- ❌ Cannot access: `/admin/roles`, `/admin/custom_fields/new` **Admin:** - ✅ Can access: `*` (all pages, including `/admin/roles`) @@ -1479,9 +1479,9 @@ defmodule MvWeb.Authorization do # Direct relationship: member.user_id Map.get(record, :user_id) == user.id - "Property" -> - # Need to traverse: property.member.user_id - # Note: In UI, property should have member preloaded + "CustomFieldValue" -> + # Need to traverse: custom_field_value.member.user_id + # Note: In UI, custom_field_value should have member preloaded case Map.get(record, :member) do %{user_id: member_user_id} -> member_user_id == user.id _ -> false @@ -1569,7 +1569,7 @@ end Admin <% end %> @@ -2409,8 +2409,8 @@ The `HasPermission` check extracts resource names via `Module.split() |> List.la |------------|------------------------| | `Mv.Accounts.User` | "User" | | `Mv.Membership.Member` | "Member" | -| `Mv.Membership.Property` | "Property" | -| `Mv.Membership.PropertyType` | "PropertyType" | +| `Mv.Membership.CustomFieldValue` | "CustomFieldValue" | +| `Mv.Membership.CustomField` | "CustomField" | | `Mv.Authorization.Role` | "Role" | These strings must match exactly in `PermissionSets` module. @@ -2450,7 +2450,7 @@ These strings must match exactly in `PermissionSets` module. **Integration:** - [ ] One complete user journey per role -- [ ] Cross-resource scenarios (e.g., Member -> Property) +- [ ] Cross-resource scenarios (e.g., Member -> CustomFieldValue) - [ ] Special cases in context (e.g., linked member email during full edit flow) ### Useful Commands diff --git a/docs/roles-and-permissions-implementation-plan.md b/docs/roles-and-permissions-implementation-plan.md index 0b173fa..2c29b8d 100644 --- a/docs/roles-and-permissions-implementation-plan.md +++ b/docs/roles-and-permissions-implementation-plan.md @@ -53,7 +53,7 @@ This document defines the implementation plan for the **MVP (Phase 1)** of the R Hardcoded in `Mv.Authorization.PermissionSets` module: 1. **own_data** - User can only access their own data (default for "Mitglied") -2. **read_only** - Read access to all members/properties (for "Vorstand", "Buchhaltung") +2. **read_only** - Read access to all members/custom field values (for "Vorstand", "Buchhaltung") 3. **normal_user** - Create/Read/Update members (no delete), full CRUD on properties (for "Kassenwart") 4. **admin** - Unrestricted access including user/role management (for "Admin") @@ -77,7 +77,7 @@ Stored in database `roles` table, each referencing a `permission_set_name`: - ✅ Hardcoded PermissionSets module with 4 permission sets - ✅ Role database table and CRUD interface - ✅ Custom Ash Policy Check (`HasPermission`) that reads from PermissionSets -- ✅ Policies on all resources (Member, User, Property, PropertyType, Role) +- ✅ Policies on all resources (Member, User, CustomFieldValue, CustomField, Role) - ✅ Page-level permissions via Phoenix Plug - ✅ UI authorization helpers for conditional rendering - ✅ Special case: Member email validation for linked users @@ -228,32 +228,32 @@ Create the core `PermissionSets` module that defines all four permission sets wi - Resources: - User: read/update :own - Member: read/update :linked - - Property: read/update :linked - - PropertyType: read :all + - CustomFieldValue: read/update :linked + - CustomField: read :all - Pages: `["/", "/profile", "/members/:id"]` **2. read_only (Vorstand, Buchhaltung):** - Resources: - User: read :own, update :own - Member: read :all - - Property: read :all - - PropertyType: read :all -- Pages: `["/", "/members", "/members/:id", "/properties"]` + - CustomFieldValue: read :all + - CustomField: read :all +- Pages: `["/", "/members", "/members/:id", "/custom_field_values"]` **3. normal_user (Kassenwart):** - Resources: - User: read/update :own - Member: read/create/update :all (no destroy for safety) - - Property: read/create/update/destroy :all - - PropertyType: read :all -- Pages: `["/", "/members", "/members/new", "/members/:id", "/members/:id/edit", "/properties", "/properties/new", "/properties/:id/edit"]` + - CustomFieldValue: read/create/update/destroy :all + - CustomField: read :all +- Pages: `["/", "/members", "/members/new", "/members/:id", "/members/:id/edit", "/custom_field_values", "/custom_field_values/new", "/custom_field_values/:id/edit"]` **4. admin:** - Resources: - User: read/update/destroy :all - Member: read/create/update/destroy :all - - Property: read/create/update/destroy :all - - PropertyType: read/create/update/destroy :all + - CustomFieldValue: read/create/update/destroy :all + - CustomField: read/create/update/destroy :all - Role: read/create/update/destroy :all - Pages: `["*"]` (wildcard = all pages) @@ -276,10 +276,10 @@ Create the core `PermissionSets` module that defines all four permission sets wi **Permission Content Tests:** - `:own_data` allows User read/update with scope :own -- `:own_data` allows Member/Property read/update with scope :linked -- `:read_only` allows Member/Property read with scope :all -- `:read_only` does NOT allow Member/Property create/update/destroy -- `:normal_user` allows Member/Property full CRUD with scope :all +- `:own_data` allows Member/CustomFieldValue read/update with scope :linked +- `:read_only` allows Member/CustomFieldValue read with scope :all +- `:read_only` does NOT allow Member/CustomFieldValue create/update/destroy +- `:normal_user` allows Member/CustomFieldValue full CRUD with scope :all - `:admin` allows everything with scope :all - `:admin` has wildcard page permission "*" @@ -387,7 +387,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss - `:own` → `{:filter, expr(id == ^actor.id)}` - `:linked` → resource-specific logic: - Member: `{:filter, expr(user_id == ^actor.id)}` - - Property: `{:filter, expr(member.user_id == ^actor.id)}` (traverse relationship!) + - CustomFieldValue: `{:filter, expr(member.user_id == ^actor.id)}` (traverse relationship!) 6. Handle errors gracefully: - No actor → `{:error, :no_actor}` - No role → `{:error, :no_role}` @@ -401,7 +401,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss - [ ] Check module implements `Ash.Policy.Check` behavior - [ ] `match?/3` correctly evaluates permissions from PermissionSets - [ ] Scope filters work correctly (:all, :own, :linked) -- [ ] `:linked` scope handles Member and Property differently +- [ ] `:linked` scope handles Member and CustomFieldValue differently - [ ] Errors are handled gracefully (no crashes) - [ ] Authorization failures are logged - [ ] Module is well-documented @@ -425,7 +425,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss **Scope Application Tests - :linked:** - Actor with scope :linked can access Member where member.user_id == actor.id -- Actor with scope :linked can access Property where property.member.user_id == actor.id (relationship traversal!) +- Actor with scope :linked can access CustomFieldValue where custom_field_value.member.user_id == actor.id (relationship traversal!) - Actor with scope :linked cannot access unlinked member - Query correctly filters based on user_id relationship @@ -581,7 +581,7 @@ Add authorization policies to the User resource. Special case: Users can always --- -#### Issue #9: Property Resource Policies +#### Issue #9: CustomFieldValue Resource Policies **Size:** M (2 days) **Dependencies:** #6 (HasPermission check) @@ -590,20 +590,20 @@ Add authorization policies to the User resource. Special case: Users can always **Description:** -Add authorization policies to the Property resource. Properties are linked to members, which are linked to users. +Add authorization policies to the CustomFieldValue resource. CustomFieldValues are linked to members, which are linked to users. **Tasks:** -1. Open `lib/mv/membership/property.ex` +1. Open `lib/mv/membership/custom_field_value.ex` 2. Add `policies` block -3. Add special policy: Allow user to read/update properties of their linked member +3. Add special policy: Allow user to read/update custom field values of their linked member ```elixir policy action_type([:read, :update]) do authorize_if expr(member.user_id == ^actor(:id)) end ``` 4. Add general policy: Check HasPermission -5. Ensure Property preloads :member relationship for scope checks +5. Ensure CustomFieldValue preloads :member relationship for scope checks 6. Preload :role relationship for actor **Policy Order:** @@ -620,27 +620,27 @@ Add authorization policies to the Property resource. Properties are linked to me **Test Strategy (TDD):** -**Linked Properties Tests (:own_data):** -- User can read properties of their linked member -- User can update properties of their linked member -- User cannot read properties of unlinked members -- Verify relationship traversal works (property.member.user_id) +**Linked CustomFieldValues Tests (:own_data):** +- User can read custom field values of their linked member +- User can update custom field values of their linked member +- User cannot read custom field values of unlinked members +- Verify relationship traversal works (custom_field_value.member.user_id) **Read-Only Tests:** -- User with :read_only can read all properties -- User with :read_only cannot create/update properties +- User with :read_only can read all custom field values +- User with :read_only cannot create/update custom field values **Normal User Tests:** -- User with :normal_user can CRUD properties +- User with :normal_user can CRUD custom field values **Admin Tests:** - Admin can perform all operations -**Test File:** `test/mv/membership/property_policies_test.exs` +**Test File:** `test/mv/membership/custom_field_value_policies_test.exs` --- -#### Issue #10: PropertyType Resource Policies +#### Issue #10: CustomField Resource Policies **Size:** S (1 day) **Dependencies:** #6 (HasPermission check) @@ -649,11 +649,11 @@ Add authorization policies to the Property resource. Properties are linked to me **Description:** -Add authorization policies to the PropertyType resource. PropertyTypes are admin-managed, but readable by all. +Add authorization policies to the CustomField resource. CustomFields are admin-managed, but readable by all. **Tasks:** -1. Open `lib/mv/membership/property_type.ex` +1. Open `lib/mv/membership/custom_field.ex` 2. Add `policies` block 3. Add read policy: All authenticated users can read (scope :all) 4. Add write policies: Only admin can create/update/destroy @@ -661,27 +661,27 @@ Add authorization policies to the PropertyType resource. PropertyTypes are admin **Acceptance Criteria:** -- [ ] All users can read property types -- [ ] Only admin can create/update/destroy property types +- [ ] All users can read custom fields +- [ ] Only admin can create/update/destroy custom fields - [ ] Policies tested **Test Strategy (TDD):** **Read Access (All Roles):** -- User with :own_data can read all property types -- User with :read_only can read all property types -- User with :normal_user can read all property types -- User with :admin can read all property types +- User with :own_data can read all custom fields +- User with :read_only can read all custom fields +- User with :normal_user can read all custom fields +- User with :admin can read all custom fields **Write Access (Admin Only):** -- Non-admin cannot create property type (Forbidden) -- Non-admin cannot update property type (Forbidden) -- Non-admin cannot destroy property type (Forbidden) -- Admin can create property type -- Admin can update property type -- Admin can destroy property type +- Non-admin cannot create custom field (Forbidden) +- Non-admin cannot update custom field (Forbidden) +- Non-admin cannot destroy custom field (Forbidden) +- Admin can create custom field +- Admin can update custom field +- Admin can destroy custom field -**Test File:** `test/mv/membership/property_type_policies_test.exs` +**Test File:** `test/mv/membership/custom_field_policies_test.exs` --- @@ -924,7 +924,7 @@ Create helper functions for UI-level authorization checks. These will be used in ``` 5. All functions use `PermissionSets.get_permissions/1` (same logic as HasPermission) 6. All functions handle nil user gracefully (return false) -7. Implement resource-specific scope checking (Member vs Property for :linked) +7. Implement resource-specific scope checking (Member vs CustomFieldValue for :linked) 8. Add comprehensive `@doc` with template examples 9. Import helper in `mv_web.ex` `html_helpers` section @@ -957,9 +957,9 @@ Create helper functions for UI-level authorization checks. These will be used in **can?/3 with Record Struct - Scope :linked:** - User can update linked Member (member.user_id == user.id) - User cannot update unlinked Member -- User can update Property of linked Member (property.member.user_id == user.id) -- User cannot update Property of unlinked Member -- Scope checking is resource-specific (Member vs Property) +- User can update CustomFieldValue of linked Member (custom_field_value.member.user_id == user.id) +- User cannot update CustomFieldValue of unlinked Member +- Scope checking is resource-specific (Member vs CustomFieldValue) **can_access_page?/2:** - User with page in list can access (returns true) @@ -1046,7 +1046,7 @@ Update Role management LiveViews to use authorization helpers for conditional re **Description:** -Update all existing LiveViews (Member, User, Property, PropertyType) to use authorization helpers for conditional rendering. +Update all existing LiveViews (Member, User, CustomFieldValue, CustomField) to use authorization helpers for conditional rendering. **Tasks:** @@ -1061,10 +1061,10 @@ Update all existing LiveViews (Member, User, Property, PropertyType) to use auth - Show: Only show other users if admin, always show own profile - Edit: Only allow editing own profile or admin editing anyone -3. **Property LiveViews:** +3. **CustomFieldValue LiveViews:** - Similar to Member (hide create/edit/delete based on permissions) -4. **PropertyType LiveViews:** +4. **CustomField LiveViews:** - All users can view - Only admin can create/edit/delete @@ -1110,13 +1110,13 @@ Update all existing LiveViews (Member, User, Property, PropertyType) to use auth - Vorstand: Sees "Home", "Members" (read-only), "Profile" - Kassenwart: Sees "Home", "Members", "Properties", "Profile" - Buchhaltung: Sees "Home", "Members" (read-only), "Profile" -- Admin: Sees "Home", "Members", "Properties", "Property Types", "Admin", "Profile" +- Admin: Sees "Home", "Members", "Custom Field Values", "Custom Fields", "Admin", "Profile" **Test Files:** - `test/mv_web/live/member_live_authorization_test.exs` - `test/mv_web/live/user_live_authorization_test.exs` -- `test/mv_web/live/property_live_authorization_test.exs` -- `test/mv_web/live/property_type_live_authorization_test.exs` +- `test/mv_web/live/custom_field_value_live_authorization_test.exs` +- `test/mv_web/live/custom_field_live_authorization_test.exs` - `test/mv_web/components/navbar_authorization_test.exs` --- @@ -1192,7 +1192,7 @@ Write comprehensive integration tests that follow complete user journeys for eac 4. Can edit any member (except email if linked - see special case) 5. Cannot delete member 6. Can manage properties -7. Cannot manage property types (read-only) +7. Cannot manage custom fields (read-only) 8. Cannot access /admin/roles **Buchhaltung Journey:** @@ -1266,7 +1266,7 @@ Write comprehensive integration tests that follow complete user journeys for eac │ │ │ ┌────▼─────┐ ┌──────▼──────┐ │ │ Issue #9 │ │ Issue #10 │ │ - │ Property │ │ PropType │ │ + │ CustomFieldValue │ │ CustomField │ │ │ Policies │ │ Policies │ │ └────┬─────┘ └──────┬──────┘ │ │ │ │ @@ -1384,8 +1384,8 @@ test/ ├── mv/membership/ │ ├── member_policies_test.exs # Issue #7 │ ├── member_email_validation_test.exs # Issue #12 -│ ├── property_policies_test.exs # Issue #9 -│ └── property_type_policies_test.exs # Issue #10 +│ ├── custom_field_value_policies_test.exs # Issue #9 +│ └── custom_field_policies_test.exs # Issue #10 ├── mv_web/ │ ├── authorization_test.exs # Issue #14 │ ├── plugs/ @@ -1395,8 +1395,8 @@ test/ │ ├── role_live_authorization_test.exs # Issue #15 │ ├── member_live_authorization_test.exs # Issue #16 │ ├── user_live_authorization_test.exs # Issue #16 -│ ├── property_live_authorization_test.exs # Issue #16 -│ └── property_type_live_authorization_test.exs # Issue #16 +│ ├── custom_field_value_live_authorization_test.exs # Issue #16 +│ └── custom_field_live_authorization_test.exs # Issue #16 ├── integration/ │ ├── mitglied_journey_test.exs # Issue #17 │ ├── vorstand_journey_test.exs # Issue #17 diff --git a/docs/roles-and-permissions-overview.md b/docs/roles-and-permissions-overview.md index 191e8b7..86e7273 100644 --- a/docs/roles-and-permissions-overview.md +++ b/docs/roles-and-permissions-overview.md @@ -201,7 +201,7 @@ When runtime permission editing becomes a business requirement, migrate to Appro **Resource Level (MVP):** - Controls create, read, update, destroy actions on resources -- Resources: Member, User, Property, PropertyType, Role +- Resources: Member, User, CustomFieldValue, CustomField, Role **Page Level (MVP):** - Controls access to LiveView pages @@ -280,7 +280,7 @@ Contains: Each Permission Set contains: **Resources:** List of resource permissions -- resource: "Member", "User", "Property", etc. +- resource: "Member", "User", "CustomFieldValue", etc. - action: :read, :create, :update, :destroy - scope: :own, :linked, :all - granted: true/false diff --git a/docs/test-status-membership-fee-ui.md b/docs/test-status-membership-fee-ui.md new file mode 100644 index 0000000..63445fb --- /dev/null +++ b/docs/test-status-membership-fee-ui.md @@ -0,0 +1,137 @@ +# Test Status: Membership Fee UI Components + +**Date:** 2025-01-XX +**Status:** Tests Written - Implementation Complete + +## Übersicht + +Alle Tests für die Membership Fee UI-Komponenten wurden geschrieben. Die Tests sind TDD-konform geschrieben und sollten erfolgreich laufen, da die Implementation bereits vorhanden ist. + +## Test-Dateien + +### Helper Module Tests + +**Datei:** `test/mv_web/helpers/membership_fee_helpers_test.exs` +- ✅ format_currency/1 formats correctly +- ✅ format_interval/1 formats all interval types +- ✅ format_cycle_range/2 formats date ranges correctly +- ✅ get_last_completed_cycle/2 returns correct cycle +- ✅ get_current_cycle/2 returns correct cycle +- ✅ status_color/1 returns correct color classes +- ✅ status_icon/1 returns correct icon names + +**Status:** Alle Tests sollten erfolgreich sein (Implementation vorhanden) + +**Datei:** `test/mv_web/member_live/index/membership_fee_status_test.exs` +- ✅ load_cycles_for_members/2 efficiently loads cycles +- ✅ get_cycle_status_for_member/2 returns correct status +- ✅ format_cycle_status_badge/1 returns correct badge + +**Status:** Alle Tests sollten erfolgreich sein (Implementation vorhanden) + +### Member List View Tests + +**Datei:** `test/mv_web/member_live/index_membership_fee_status_test.exs` +- ✅ Status column displays correctly +- ✅ Shows last completed cycle status by default +- ✅ Toggle switches to current cycle view +- ✅ Color coding for paid/unpaid/suspended +- ✅ Filter "Unpaid in last cycle" works +- ✅ Filter "Unpaid in current cycle" works +- ✅ Handles members without cycles gracefully +- ✅ Loads cycles efficiently without N+1 queries + +**Status:** Alle Tests sollten erfolgreich sein (Implementation vorhanden) + +### Member Detail View Tests + +**Datei:** `test/mv_web/member_live/show_membership_fees_test.exs` +- ✅ Cycles table displays all cycles +- ✅ Table columns show correct data +- ✅ Membership fee type dropdown shows only same-interval types +- ✅ Warning displayed if different interval selected +- ✅ Status change actions work (mark as paid/suspended/unpaid) +- ✅ Cycle regeneration works +- ✅ Handles members without membership fee type gracefully + +**Status:** Alle Tests sollten erfolgreich sein (Implementation vorhanden) + +### Membership Fee Types Admin Tests + +**Datei:** `test/mv_web/live/membership_fee_type_live/index_test.exs` +- ✅ List displays all types with correct data +- ✅ Member count column shows correct count +- ✅ Create button navigates to form +- ✅ Edit button per row navigates to edit form +- ✅ Delete button disabled if type is in use +- ✅ Delete button works if type is not in use +- ✅ Only admin can access + +**Status:** Alle Tests sollten erfolgreich sein (Implementation vorhanden) + +**Datei:** `test/mv_web/live/membership_fee_type_live/form_test.exs` +- ✅ Create form works +- ✅ Edit form loads existing type data +- ✅ Interval field editable on create +- ✅ Interval field grayed out on edit +- ✅ Amount change warning displays on edit +- ✅ Amount change warning shows correct affected member count +- ✅ Amount change can be confirmed +- ✅ Amount change can be cancelled +- ✅ Validation errors display correctly +- ✅ Only admin can access + +**Status:** Alle Tests sollten erfolgreich sein (Implementation vorhanden) + +### Member Form Tests + +**Datei:** `test/mv_web/member_live/form_membership_fee_type_test.exs` +- ✅ Membership fee type dropdown displays in form +- ✅ Shows available types +- ✅ Filters to same interval types if member has type +- ✅ Warning displayed if different interval selected +- ✅ Warning cleared if same interval selected +- ✅ Form saves with selected membership fee type +- ✅ New members get default membership fee type + +**Status:** Alle Tests sollten erfolgreich sein (Implementation vorhanden) + +### Integration Tests + +**Datei:** `test/mv_web/member_live/membership_fee_integration_test.exs` +- ✅ End-to-end: Create type → Assign to member → View cycles → Change status +- ✅ End-to-end: Change member type → Cycles regenerate +- ✅ End-to-end: Update settings → New members get default type +- ✅ End-to-end: Delete cycle → Confirmation → Cycle deleted +- ✅ End-to-end: Edit cycle amount → Modal → Amount updated + +**Status:** Alle Tests sollten erfolgreich sein (Implementation vorhanden) + +## Test-Ausführung + +Alle Tests können mit folgenden Befehlen ausgeführt werden: + +```bash +# Alle Tests +mix test + +# Nur Membership Fee Tests +mix test test/mv_web/helpers/membership_fee_helpers_test.exs +mix test test/mv_web/member_live/ +mix test test/mv_web/live/membership_fee_type_live/ + +# Mit Coverage +mix test --cover +``` + +## Bekannte Probleme + +Keine bekannten Probleme. Alle Tests sollten erfolgreich laufen, da die Implementation bereits vorhanden ist. + +## Nächste Schritte + +1. ✅ Tests geschrieben +2. ⏳ Tests ausführen und verifizieren +3. ⏳ Eventuelle Anpassungen basierend auf Test-Ergebnissen +4. ⏳ Code-Review durchführen + diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index dbc62b2..ceedeae 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -17,6 +17,10 @@ defmodule Mv.Accounts.User do # When a member is deleted, set the user's member_id to NULL # This allows users to continue existing even if their linked member is removed reference :member, on_delete: :nilify + + # When a role is deleted, prevent deletion if users are assigned to it + # This protects critical roles from accidental deletion + reference :role, on_delete: :restrict end end @@ -357,6 +361,12 @@ defmodule Mv.Accounts.User do # This automatically creates a `member_id` attribute in the User table # The relationship is optional (allow_nil? true by default) belongs_to :member, Mv.Membership.Member + + # 1:1 relationship - User belongs to a Role + # This automatically creates a `role_id` attribute in the User table + # The relationship is optional (allow_nil? true by default) + # Foreign key constraint: on_delete: :restrict (prevents deleting roles assigned to users) + belongs_to :role, Mv.Authorization.Role end identities do diff --git a/lib/membership/custom_field.ex b/lib/membership/custom_field.ex index 5b7514c..18b8154 100644 --- a/lib/membership/custom_field.ex +++ b/lib/membership/custom_field.ex @@ -12,7 +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`) - `description` - Optional human-readable description - - `immutable` - If true, custom field values cannot be changed after creation - `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 @@ -60,10 +59,10 @@ defmodule Mv.Membership.CustomField do actions do defaults [:read, :update] - default_accept [:name, :value_type, :description, :immutable, :required, :show_in_overview] + default_accept [:name, :value_type, :description, :required, :show_in_overview] create :create do - accept [:name, :value_type, :description, :immutable, :required, :show_in_overview] + accept [:name, :value_type, :description, :required, :show_in_overview] change Mv.Membership.CustomField.Changes.GenerateSlug validate string_length(:slug, min: 1) end @@ -113,10 +112,6 @@ defmodule Mv.Membership.CustomField do trim?: true ] - attribute :immutable, :boolean, - default: false, - allow_nil?: false - attribute :required, :boolean, default: false, allow_nil?: false diff --git a/lib/membership/member.ex b/lib/membership/member.ex index b788dc9..1d6d96e 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -29,7 +29,9 @@ defmodule Mv.Membership.Member do ## Full-Text Search Members have a `search_vector` attribute (tsvector) that is automatically - updated via database trigger. Search includes name, email, notes, and contact fields. + updated via database trigger. Search includes name, email, notes, contact fields, + and all custom field values. Custom field values are automatically included in + the search vector with weight 'C' (same as phone_number, city, etc.). """ use Ash.Resource, domain: Mv.Membership, @@ -37,9 +39,25 @@ defmodule Mv.Membership.Member do require Ash.Query import Ash.Expr + require Logger # Module constants @member_search_limit 10 + + # Similarity threshold for fuzzy name/address matching. + # Lower value = more results but less accurate (0.1-0.9) + # + # Fuzzy matching uses two complementary strategies: + # 1. % operator: Fast GIN-index-based matching using server-wide threshold (default 0.3) + # - Catches exact trigram matches quickly via index + # 2. similarity/word_similarity functions: Precise matching with this configurable threshold + # - Catches partial matches that % operator might miss + # + # Value 0.2 chosen based on testing with typical German names: + # - "Müller" vs "Mueller": similarity ~0.65 ✓ + # - "Schmidt" vs "Schmitt": similarity ~0.75 ✓ + # - "Wagner" vs "Wegner": similarity ~0.55 ✓ + # - Random unrelated names: similarity ~0.15 ✗ @default_similarity_threshold 0.2 # Use constants from Mv.Constants for member fields @@ -56,13 +74,17 @@ defmodule Mv.Membership.Member do create :create_member do primary? true + + # Note: Custom validation function cannot be done atomically (queries DB for required custom fields) + # In Ash 3.0, require_atomic? is not available for create actions, but the validation will still work # Custom field values can be created along with member argument :custom_field_values, {:array, :map} # Allow user to be passed as argument for relationship management # user_id is NOT in accept list to prevent direct foreign key manipulation argument :user, :map, allow_nil?: true - accept @member_fields + # Accept member fields plus membership_fee_type_id (belongs_to FK) + accept @member_fields ++ [:membership_fee_type_id, :membership_fee_start_date] change manage_relationship(:custom_field_values, type: :create) @@ -83,6 +105,31 @@ defmodule Mv.Membership.Member do change Mv.EmailSync.Changes.SyncUserEmailToMember do where [changing(:user)] end + + # Auto-assign default membership fee type if not explicitly set + change Mv.Membership.Member.Changes.SetDefaultMembershipFeeType + + # Auto-calculate membership_fee_start_date if not manually set + # Requires both join_date and membership_fee_type_id to be present + change Mv.MembershipFees.Changes.SetMembershipFeeStartDate + + # Trigger cycle generation after member creation + # Only runs if membership_fee_type_id is set + # Note: Cycle generation runs asynchronously to not block the action, + # but in test environment it runs synchronously for DB sandbox compatibility + change after_transaction(fn _changeset, result, _context -> + case result do + {:ok, member} -> + if member.membership_fee_type_id && member.join_date do + handle_cycle_generation(member) + end + + {:error, _} -> + :ok + end + + result + end) end update :update_member do @@ -95,7 +142,8 @@ defmodule Mv.Membership.Member do # user_id is NOT in accept list to prevent direct foreign key manipulation argument :user, :map, allow_nil?: true - accept @member_fields + # Accept member fields plus membership_fee_type_id (belongs_to FK) + accept @member_fields ++ [:membership_fee_type_id, :membership_fee_start_date] change manage_relationship(:custom_field_values, on_match: :update, on_no_match: :create) @@ -122,6 +170,69 @@ defmodule Mv.Membership.Member do change Mv.EmailSync.Changes.SyncUserEmailToMember do where [changing(:user)] end + + # Validate that membership fee type changes only allow same-interval types + change Mv.MembershipFees.Changes.ValidateSameInterval do + where [changing(:membership_fee_type_id)] + end + + # Auto-calculate membership_fee_start_date when membership_fee_type_id is set + # and membership_fee_start_date is not already set + change Mv.MembershipFees.Changes.SetMembershipFeeStartDate do + where [changing(:membership_fee_type_id)] + end + + # Trigger cycle regeneration when membership_fee_type_id changes + # This deletes future unpaid cycles and regenerates them with the new type/amount + # Note: Cycle regeneration runs synchronously in the same transaction to ensure atomicity + # CycleGenerator uses advisory locks and transactions internally to prevent race conditions + # Notifications are returned to Ash and sent automatically after commit + change after_action(fn changeset, member, _context -> + fee_type_changed = + Ash.Changeset.changing_attribute?(changeset, :membership_fee_type_id) + + if fee_type_changed && member.membership_fee_type_id && member.join_date do + case regenerate_cycles_on_type_change(member) do + {:ok, notifications} -> + # Return notifications to Ash - they will be sent automatically after commit + {:ok, member, notifications} + + {:error, reason} -> + require Logger + + Logger.warning( + "Failed to regenerate cycles for member #{member.id}: #{inspect(reason)}" + ) + + {:ok, member} + end + else + {:ok, member} + end + end) + + # Trigger cycle regeneration when join_date or exit_date changes + # Regenerates cycles based on new dates + # Note: Cycle generation runs synchronously in test environment, asynchronously in production + # CycleGenerator uses advisory locks and transactions internally to prevent race conditions + # If both join_date and exit_date are changed simultaneously, this hook runs only once + # (Ash ensures each after_transaction hook runs once per action, regardless of how many attributes changed) + change after_transaction(fn changeset, result, _context -> + case result do + {:ok, member} -> + join_date_changed = Ash.Changeset.changing_attribute?(changeset, :join_date) + exit_date_changed = Ash.Changeset.changing_attribute?(changeset, :exit_date) + + if (join_date_changed || exit_date_changed) && member.membership_fee_type_id do + handle_cycle_generation(member) + end + + {:error, _} -> + :ok + end + + result + end) end # Action to handle fuzzy search on specific fields @@ -139,30 +250,21 @@ defmodule Mv.Membership.Member do if is_binary(q) and String.trim(q) != "" do q2 = String.trim(q) - pat = "%" <> q2 <> "%" + # Sanitize for LIKE patterns (escape % and _), limit length to 100 chars + q2_sanitized = sanitize_search_query(q2) + pat = "%" <> q2_sanitized <> "%" + + # Build search filters grouped by search type for maintainability + # Priority: FTS > Substring > Custom Fields > Fuzzy Matching + # Note: FTS and fuzzy use q2 (unsanitized), LIKE-based filters use pat (sanitized) + fts_match = build_fts_filter(q2) + substring_match = build_substring_filter(q2_sanitized, pat) + custom_field_match = build_custom_field_filter(pat) + fuzzy_match = build_fuzzy_filter(q2, threshold) - # FTS as main filter and fuzzy search just for first name, last name and strees query |> Ash.Query.filter( - expr( - # Substring on numeric-like fields (best effort, supports middle substrings) - fragment("search_vector @@ websearch_to_tsquery('simple', ?)", ^q2) or - fragment("search_vector @@ plainto_tsquery('simple', ?)", ^q2) or - contains(postal_code, ^q2) or - contains(house_number, ^q2) or - contains(phone_number, ^q2) or - contains(email, ^q2) or - contains(city, ^q2) or ilike(city, ^pat) or - fragment("? % first_name", ^q2) or - fragment("? % last_name", ^q2) or - fragment("? % street", ^q2) or - fragment("word_similarity(?, first_name) > ?", ^q2, ^threshold) or - fragment("word_similarity(?, last_name) > ?", ^q2, ^threshold) or - fragment("word_similarity(?, street) > ?", ^q2, ^threshold) or - fragment("similarity(first_name, ?) > ?", ^q2, ^threshold) or - fragment("similarity(last_name, ?) > ?", ^q2, ^threshold) or - fragment("similarity(street, ?) > ?", ^q2, ^threshold) - ) + expr(^fts_match or ^substring_match or ^custom_field_match or ^fuzzy_match) ) else query @@ -284,7 +386,7 @@ defmodule Mv.Membership.Member do end end - # Join date not in the future + # 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" @@ -319,6 +421,32 @@ defmodule Mv.Membership.Member do {:error, field: :email, message: "is not a valid email"} end end + + # Validate required custom fields + validate fn changeset, _ -> + provided_values = provided_custom_field_values(changeset) + + case Mv.Membership.list_required_custom_fields() do + {:ok, required_custom_fields} -> + missing_fields = missing_required_fields(required_custom_fields, provided_values) + + if Enum.empty?(missing_fields) do + :ok + else + build_custom_field_validation_error(missing_fields) + end + + {:error, error} -> + Logger.error( + "Failed to load custom fields for validation: #{inspect(error)}. Required field validation cannot be performed." + ) + + {:error, + field: :custom_field_values, + message: + "Unable to validate required custom fields. Please try again or contact support."} + end + end end attributes do @@ -346,10 +474,6 @@ defmodule Mv.Membership.Member do constraints min_length: 5, max_length: 254 end - attribute :paid, :boolean do - allow_nil? true - end - attribute :phone_number, :string do allow_nil? true end @@ -386,6 +510,15 @@ defmodule Mv.Membership.Member do writable?: false, public?: false, select_by_default?: false + + # Membership fee fields + # membership_fee_start_date: Date from which membership fees should be calculated + # If nil, calculated from join_date + global setting + attribute :membership_fee_start_date, :date do + allow_nil? true + public? true + description "Date from which membership fees should be calculated" + end end relationships do @@ -394,6 +527,60 @@ defmodule Mv.Membership.Member do # This references the User's member_id attribute # The relationship is optional (allow_nil? true by default) has_one :user, Mv.Accounts.User + + # Membership fee relationships + # belongs_to: The fee type assigned to this member + # Optional for MVP - can be nil if no fee type assigned yet + belongs_to :membership_fee_type, Mv.MembershipFees.MembershipFeeType do + allow_nil? true + end + + # has_many: All fee cycles for this member + has_many :membership_fee_cycles, Mv.MembershipFees.MembershipFeeCycle + end + + calculations do + calculate :current_cycle_status, :atom do + description "Status of the current cycle (the one that is active today)" + # Automatically load cycles with all attributes and membership_fee_type + load membership_fee_cycles: [:cycle_start, :status, membership_fee_type: [:interval]] + + calculation fn [member], _context -> + case get_current_cycle(member) do + nil -> [nil] + cycle -> [cycle.status] + end + end + + constraints one_of: [:unpaid, :paid, :suspended] + end + + calculate :last_cycle_status, :atom do + description "Status of the last completed cycle (the most recent cycle that has ended)" + # Automatically load cycles with all attributes and membership_fee_type + load membership_fee_cycles: [:cycle_start, :status, membership_fee_type: [:interval]] + + calculation fn [member], _context -> + case get_last_completed_cycle(member) do + nil -> [nil] + cycle -> [cycle.status] + end + end + + constraints one_of: [:unpaid, :paid, :suspended] + end + + calculate :overdue_count, :integer do + description "Count of unpaid cycles that have already ended (cycle_end < today)" + # Automatically load cycles with all attributes and membership_fee_type + load membership_fee_cycles: [:cycle_start, :status, membership_fee_type: [:interval]] + + calculation fn [member], _context -> + overdue = get_overdue_cycles(member) + count = if is_list(overdue), do: length(overdue), else: 0 + [count] + end + end end # Define identities for upsert operations @@ -442,6 +629,345 @@ defmodule Mv.Membership.Member do def show_in_overview?(_), do: true + # Helper functions for cycle status calculations + # + # These functions expect membership_fee_cycles to be loaded with membership_fee_type + # preloaded. The calculations explicitly load this relationship, but if called + # directly, ensure membership_fee_type is loaded or the functions will return + # nil/[] when membership_fee_type is missing. + + @doc false + @spec get_current_cycle(Member.t()) :: MembershipFeeCycle.t() | nil + def get_current_cycle(member) do + today = Date.utc_today() + + # Check if cycles are already loaded + cycles = Map.get(member, :membership_fee_cycles) + + if is_list(cycles) and cycles != [] do + Enum.find(cycles, ¤t_cycle?(&1, today)) + else + nil + end + end + + # Checks if a cycle is the current cycle (active today) + defp current_cycle?(cycle, today) do + case Map.get(cycle, :membership_fee_type) do + %{interval: interval} -> + cycle_end = + Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + + Date.compare(cycle.cycle_start, today) in [:lt, :eq] and + Date.compare(today, cycle_end) in [:lt, :eq] + + _ -> + false + end + end + + @doc false + @spec get_last_completed_cycle(Member.t()) :: MembershipFeeCycle.t() | nil + def get_last_completed_cycle(member) do + today = Date.utc_today() + + # Check if cycles are already loaded + cycles = Map.get(member, :membership_fee_cycles) + + if is_list(cycles) and cycles != [] do + cycles + |> filter_completed_cycles(today) + |> sort_cycles_by_end_date() + |> List.first() + else + nil + end + end + + # Filters cycles that have ended (cycle_end < today) + defp filter_completed_cycles(cycles, today) do + Enum.filter(cycles, fn cycle -> + case Map.get(cycle, :membership_fee_type) do + %{interval: interval} -> + cycle_end = + Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + + Date.compare(today, cycle_end) == :gt + + _ -> + false + end + end) + end + + # Sorts cycles by end date in descending order + defp sort_cycles_by_end_date(cycles) do + Enum.sort_by( + cycles, + fn cycle -> + interval = Map.get(cycle, :membership_fee_type).interval + Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + end, + {:desc, Date} + ) + end + + @doc false + @spec get_overdue_cycles(Member.t()) :: [MembershipFeeCycle.t()] + def get_overdue_cycles(member) do + today = Date.utc_today() + + # Check if cycles are already loaded + cycles = Map.get(member, :membership_fee_cycles) + + if is_list(cycles) and cycles != [] do + filter_overdue_cycles(cycles, today) + else + [] + end + end + + # Filters cycles that are unpaid and have ended (cycle_end < today) + defp filter_overdue_cycles(cycles, today) do + Enum.filter(cycles, fn cycle -> + case Map.get(cycle, :membership_fee_type) do + %{interval: interval} -> + cycle_end = + Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + + cycle.status == :unpaid and Date.compare(today, cycle_end) == :gt + + _ -> + false + end + end) + end + + # Regenerates cycles when membership fee type changes + # Deletes future unpaid cycles and regenerates them with the new type/amount + # Uses advisory lock to prevent concurrent modifications + # Returns {:ok, notifications} or {:error, reason} where notifications are collected + # to be sent after transaction commits + @doc false + def regenerate_cycles_on_type_change(member) do + today = Date.utc_today() + lock_key = :erlang.phash2(member.id) + + # Use advisory lock to prevent concurrent deletion and regeneration + # This ensures atomicity when multiple updates happen simultaneously + if Mv.Repo.in_transaction?() do + regenerate_cycles_in_transaction(member, today, lock_key) + else + regenerate_cycles_new_transaction(member, today, lock_key) + end + end + + # 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 + Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) + end + + # Not in transaction: start new transaction with advisory lock + # Returns {:ok, notifications} - notifications should be sent by caller (e.g., via after_action) + defp regenerate_cycles_new_transaction(member, today, lock_key) do + Mv.Repo.transaction(fn -> + Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + + case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) do + {:ok, notifications} -> + # Return notifications - they will be sent by the caller + notifications + + {:error, reason} -> + Mv.Repo.rollback(reason) + end + end) + |> case do + {:ok, notifications} -> {:ok, notifications} + {:error, reason} -> {:error, reason} + end + end + + # Performs the actual cycle deletion and regeneration + # Returns {:ok, notifications} or {:error, reason} + # notifications are collected to be sent after transaction commits + defp do_regenerate_cycles_on_type_change(member, today, opts) do + require Ash.Query + + skip_lock? = Keyword.get(opts, :skip_lock?, false) + + # Find all unpaid cycles for this member + # We need to check cycle_end for each cycle using its own interval + all_unpaid_cycles_query = + Mv.MembershipFees.MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member.id) + |> Ash.Query.filter(status == :unpaid) + |> Ash.Query.load([:membership_fee_type]) + + case Ash.read(all_unpaid_cycles_query) do + {:ok, all_unpaid_cycles} -> + cycles_to_delete = filter_future_cycles(all_unpaid_cycles, today) + delete_and_regenerate_cycles(cycles_to_delete, member.id, today, skip_lock?: skip_lock?) + + {:error, reason} -> + {:error, reason} + end + end + + # Filters cycles that haven't ended yet (cycle_end >= today) + # These are the "future" cycles that should be regenerated + defp filter_future_cycles(all_unpaid_cycles, today) do + Enum.filter(all_unpaid_cycles, fn cycle -> + case cycle.membership_fee_type do + %{interval: interval} -> + cycle_end = + Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + + Date.compare(today, cycle_end) in [:lt, :eq] + + _ -> + false + end + end) + end + + # Deletes future cycles and regenerates them with the new type/amount + # Passes today to ensure consistent date across deletion and regeneration + # Returns {:ok, notifications} or {:error, reason} + defp delete_and_regenerate_cycles(cycles_to_delete, member_id, today, opts) do + skip_lock? = Keyword.get(opts, :skip_lock?, false) + + if Enum.empty?(cycles_to_delete) do + # No cycles to delete, just regenerate + regenerate_cycles(member_id, today, skip_lock?: skip_lock?) + else + case delete_cycles(cycles_to_delete) do + :ok -> regenerate_cycles(member_id, today, skip_lock?: skip_lock?) + {:error, reason} -> {:error, reason} + end + end + end + + # Deletes cycles and returns :ok if all succeeded, {:error, reason} otherwise + defp delete_cycles(cycles_to_delete) do + delete_results = + Enum.map(cycles_to_delete, fn cycle -> + Ash.destroy(cycle) + end) + + if Enum.any?(delete_results, &match?({:error, _}, &1)) do + {:error, :deletion_failed} + else + :ok + end + end + + # Regenerates cycles with new type/amount + # Passes today to ensure consistent date across deletion and regeneration + # skip_lock?: true means advisory lock is already set by caller + # Returns {:ok, notifications} - notifications should be returned to after_action hook + defp regenerate_cycles(member_id, today, opts) do + skip_lock? = Keyword.get(opts, :skip_lock?, false) + + case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member( + member_id, + today: today, + skip_lock?: skip_lock? + ) do + {:ok, _cycles, notifications} when is_list(notifications) -> + {:ok, notifications} + + {:error, reason} -> + {:error, reason} + end + end + + # Handles cycle generation for a member, choosing sync or async execution + # based on environment (test vs production) + # This function encapsulates the common logic for cycle generation + # to avoid code duplication across different hooks + defp handle_cycle_generation(member) do + if Mv.Config.sql_sandbox?() do + handle_cycle_generation_sync(member) + else + handle_cycle_generation_async(member) + end + end + + # Runs cycle generation synchronously (for test environment) + defp handle_cycle_generation_sync(member) do + require Logger + + case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member( + member.id, + today: Date.utc_today() + ) do + {:ok, cycles, notifications} -> + send_notifications_if_any(notifications) + log_cycle_generation_success(member, cycles, notifications, sync: true) + + {:error, reason} -> + log_cycle_generation_error(member, reason, sync: true) + end + end + + # Runs cycle generation asynchronously (for production environment) + defp handle_cycle_generation_async(member) do + Task.Supervisor.async_nolink(Mv.TaskSupervisor, fn -> + case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id) do + {:ok, cycles, notifications} -> + send_notifications_if_any(notifications) + log_cycle_generation_success(member, cycles, notifications, sync: false) + + {:error, reason} -> + log_cycle_generation_error(member, reason, sync: false) + end + end) + end + + # Sends notifications if any are present + defp send_notifications_if_any(notifications) do + if Enum.any?(notifications) do + Ash.Notifier.notify(notifications) + end + end + + # Logs successful cycle generation + defp log_cycle_generation_success(member, cycles, notifications, sync: sync?) do + require Logger + + sync_label = if sync?, do: "", else: " (async)" + + Logger.debug( + "Successfully generated cycles for member#{sync_label}", + member_id: member.id, + cycles_count: length(cycles), + notifications_count: length(notifications) + ) + end + + # Logs cycle generation errors + defp log_cycle_generation_error(member, reason, sync: sync?) do + require Logger + + sync_label = if sync?, do: "", else: " (async)" + + Logger.error( + "Failed to generate cycles for member#{sync_label}", + member_id: member.id, + member_email: member.email, + error: inspect(reason), + error_type: error_type(reason) + ) + end + + # Helper to extract error type for structured logging + defp error_type(%{__struct__: struct_name}), do: struct_name + defp error_type(error) when is_atom(error), do: error + defp error_type(_), do: :unknown + # Normalizes visibility config map keys from strings to atoms. # JSONB in PostgreSQL converts atom keys to string keys when storing. defp normalize_visibility_config(config) when is_map(config) do @@ -476,7 +1002,6 @@ defmodule Mv.Membership.Member do - `query` - Ash.Query.t() to apply search to - `opts` - Keyword list or map with search options: - `:query` or `"query"` - Search string - - `:fields` or `"fields"` - Optional field restrictions ## Returns - Modified Ash.Query.t() with search filters applied @@ -497,16 +1022,103 @@ defmodule Mv.Membership.Member do if String.trim(q) == "" do query else - args = - case opts[:fields] || opts["fields"] do - nil -> %{query: q} - fields -> %{query: q, fields: fields} - end - - Ash.Query.for_read(query, :search, args) + Ash.Query.for_read(query, :search, %{query: q}) end end + # ============================================================================ + # Search Input Sanitization + # ============================================================================ + + # Sanitizes search input to prevent LIKE pattern injection. + # Escapes SQL LIKE wildcards (% and _) and limits query length. + # + # ## Examples + # + # iex> sanitize_search_query("test%injection") + # "test\\%injection" + # + # iex> sanitize_search_query("very_long_search") + # "very\\_long\\_search" + # + defp sanitize_search_query(query) when is_binary(query) do + query + |> String.slice(0, 100) + |> String.replace("\\", "\\\\") + |> String.replace("%", "\\%") + |> String.replace("_", "\\_") + end + + defp sanitize_search_query(_), do: "" + + # ============================================================================ + # Search Filter Builders + # ============================================================================ + # These functions build search filters grouped by search type for maintainability. + # Priority order: FTS > Substring > Custom Fields > Fuzzy Matching + + # Builds full-text search filter using tsvector (highest priority, fastest) + # Uses GIN index on search_vector for optimal performance + defp build_fts_filter(query) do + expr( + fragment("search_vector @@ websearch_to_tsquery('simple', ?)", ^query) or + fragment("search_vector @@ plainto_tsquery('simple', ?)", ^query) + ) + end + + # Builds substring search filter for structured fields + # Note: contains/2 uses ILIKE '%value%' which is not index-optimized + # Performance: Good for small datasets, may be slow on large tables + defp build_substring_filter(query, _pattern) do + expr( + contains(postal_code, ^query) or + contains(house_number, ^query) or + contains(phone_number, ^query) or + contains(email, ^query) or + contains(city, ^query) + ) + end + + # Builds search filter for custom field values using ILIKE on JSONB + # Note: ILIKE on JSONB is not index-optimized, may be slow with many custom fields + # This is a fallback for substring matching in custom fields (e.g., phone numbers) + # Uses ->> operator which always returns TEXT directly (no need for -> + ::text fallback) + # Important: `id` must be passed as parameter to correctly reference the outer members table + defp build_custom_field_filter(pattern) do + expr( + fragment( + "EXISTS (SELECT 1 FROM custom_field_values WHERE member_id = ? AND (value->>'_union_value' ILIKE ? OR value->>'value' ILIKE ?))", + id, + ^pattern, + ^pattern + ) + ) + end + + # Builds fuzzy/trigram matching filter for name and street fields. + # Uses pg_trgm extension with GIN indexes for performance. + # + # Two-tier matching strategy: + # - % operator: Uses server-wide pg_trgm.similarity_threshold (typically 0.3) + # for fast index-based initial filtering + # - similarity/word_similarity: Uses @default_similarity_threshold (0.2) + # for more lenient matching to catch edge cases + # + # Note: Requires trigram GIN indexes on first_name, last_name, street. + defp build_fuzzy_filter(query, threshold) do + expr( + fragment("? % first_name", ^query) or + fragment("? % last_name", ^query) or + fragment("? % street", ^query) or + fragment("word_similarity(?, first_name) > ?", ^query, ^threshold) or + fragment("word_similarity(?, last_name) > ?", ^query, ^threshold) or + fragment("word_similarity(?, street) > ?", ^query, ^threshold) or + fragment("similarity(first_name, ?) > ?", ^query, ^threshold) or + fragment("similarity(last_name, ?) > ?", ^query, ^threshold) or + fragment("similarity(street, ?) > ?", ^query, ^threshold) + ) + end + # Private helper to apply filters for :available_for_linking action # user_email: may be nil/empty when creating new user, or populated when editing # search_query: optional search term for fuzzy matching @@ -515,9 +1127,9 @@ defmodule Mv.Membership.Member do # - Empty user_email ("") → email == "" is always false → only fuzzy search matches # - This allows a single filter expression instead of duplicating fuzzy search logic # - # Cyclomatic complexity is unavoidable here: PostgreSQL fuzzy search requires - # multiple OR conditions for good search quality (FTS + trigram similarity + substring) - # credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity + # Note: Custom field search is intentionally excluded from linking to optimize + # autocomplete performance. Custom fields are still searchable via the main + # member search which uses the indexed search_vector. defp apply_linking_filters(query, user_email, search_query) do has_search = search_query && String.trim(search_query) != "" # Use empty string instead of nil to simplify filter logic @@ -526,35 +1138,23 @@ defmodule Mv.Membership.Member do if has_search do # Search query provided: return email-match OR fuzzy-search candidates trimmed_search = String.trim(search_query) + # Sanitize for LIKE patterns (contains uses ILIKE internally) + sanitized_search = sanitize_search_query(trimmed_search) + + # Build search filters - excluding custom_field_filter for performance + fts_match = build_fts_filter(trimmed_search) + fuzzy_match = build_fuzzy_filter(trimmed_search, @default_similarity_threshold) + email_substring_match = expr(contains(email, ^sanitized_search)) query |> Ash.Query.filter( expr( - # Email match candidate (for filter_by_email_match priority) - # If email is "", this is always false and fuzzy search takes over - # Fuzzy search candidates + # Email exact match has highest priority (for filter_by_email_match) + # If email is "", this is always false and search filters take over email == ^trimmed_email or - fragment("search_vector @@ websearch_to_tsquery('simple', ?)", ^trimmed_search) or - fragment("search_vector @@ plainto_tsquery('simple', ?)", ^trimmed_search) or - fragment("? % first_name", ^trimmed_search) or - fragment("? % last_name", ^trimmed_search) or - fragment("word_similarity(?, first_name) > 0.2", ^trimmed_search) or - fragment( - "word_similarity(?, last_name) > ?", - ^trimmed_search, - ^@default_similarity_threshold - ) or - fragment( - "similarity(first_name, ?) > ?", - ^trimmed_search, - ^@default_similarity_threshold - ) or - fragment( - "similarity(last_name, ?) > ?", - ^trimmed_search, - ^@default_similarity_threshold - ) or - contains(email, ^trimmed_search) + ^fts_match or + ^fuzzy_match or + ^email_substring_match ) ) else @@ -562,4 +1162,127 @@ defmodule Mv.Membership.Member do query end end + + # Extracts provided custom field values from changeset + # Handles both create (from argument) and update (from existing data) scenarios + defp provided_custom_field_values(changeset) do + custom_field_values_arg = Ash.Changeset.get_argument(changeset, :custom_field_values) + + if is_nil(custom_field_values_arg) do + extract_existing_values(changeset.data) + else + extract_argument_values(custom_field_values_arg) + end + end + + # Extracts custom field values from existing member data (update scenario) + defp extract_existing_values(member_data) do + case Ash.load(member_data, :custom_field_values) do + {:ok, %{custom_field_values: existing_values}} -> + Enum.reduce(existing_values, %{}, &extract_value_from_cfv/2) + + _ -> + %{} + end + end + + # Extracts value from a CustomFieldValue struct + defp extract_value_from_cfv(cfv, acc) do + value = extract_union_value(cfv.value) + Map.put(acc, cfv.custom_field_id, value) + end + + # Extracts value from union type (map or direct value) + defp extract_union_value(value) when is_map(value), do: Map.get(value, :value) + defp extract_union_value(value), do: value + + # Extracts custom field values from provided argument (create/update scenario) + defp extract_argument_values(custom_field_values_arg) do + Enum.reduce(custom_field_values_arg, %{}, &extract_value_from_arg/2) + end + + # Extracts value from argument map + defp extract_value_from_arg(cfv, acc) do + custom_field_id = Map.get(cfv, "custom_field_id") + value_map = Map.get(cfv, "value", %{}) + actual_value = extract_value_from_map(value_map) + Map.put(acc, custom_field_id, actual_value) + end + + # Extracts value from map, supporting both "value" and "_union_value" keys + # Also handles Ash.Union structs (which have atom keys :value and :type) + # Uses cond instead of || to preserve false values + defp extract_value_from_map(value_map) do + cond do + # Handle Ash.Union struct - check if it's a struct with __struct__ == Ash.Union + match?({:ok, Ash.Union}, Map.fetch(value_map, :__struct__)) -> + Map.get(value_map, :value) + + # Handle map with string keys + Map.has_key?(value_map, "value") -> + Map.get(value_map, "value") + + Map.has_key?(value_map, "_union_value") -> + Map.get(value_map, "_union_value") + + # Handle map with atom keys + Map.has_key?(value_map, :value) -> + Map.get(value_map, :value) + + true -> + nil + end + end + + # Finds which required custom fields are missing from provided values + defp missing_required_fields(required_custom_fields, provided_values) do + Enum.filter(required_custom_fields, fn cf -> + value = Map.get(provided_values, cf.id) + not value_present?(value, cf.value_type) + end) + end + + # Builds validation error message for missing required custom fields + defp build_custom_field_validation_error(missing_fields) do + # Sort missing fields alphabetically for consistent error messages + sorted_missing_fields = Enum.sort_by(missing_fields, & &1.name) + missing_names = Enum.map_join(sorted_missing_fields, ", ", & &1.name) + + {:error, + field: :custom_field_values, + message: + Gettext.dgettext(MvWeb.Gettext, "default", "Required custom fields missing: %{fields}", + fields: missing_names + )} + end + + # Helper function to check if a value is present for a given custom field type + # Boolean: false is valid, only nil is invalid + # String: nil or empty strings are invalid + # Integer: nil or empty strings are invalid, 0 is valid + # Date: nil or empty strings are invalid + # Email: nil or empty strings are invalid + defp value_present?(nil, _type), do: false + + defp value_present?(value, :boolean), do: not is_nil(value) + + defp value_present?(value, :string), do: is_binary(value) and String.trim(value) != "" + + defp value_present?(value, :integer) when is_integer(value), do: true + + defp value_present?(value, :integer) when is_binary(value), do: String.trim(value) != "" + + defp value_present?(_value, :integer), do: false + + defp value_present?(value, :date) when is_struct(value, Date), do: true + + defp value_present?(value, :date) when is_binary(value), do: String.trim(value) != "" + + defp value_present?(_value, :date), do: false + + defp value_present?(value, :email) when is_binary(value), do: String.trim(value) != "" + + defp value_present?(_value, :email), do: false + + defp value_present?(_value, _type), do: false end diff --git a/lib/membership/member/changes/set_default_membership_fee_type.ex b/lib/membership/member/changes/set_default_membership_fee_type.ex new file mode 100644 index 0000000..8b75ed7 --- /dev/null +++ b/lib/membership/member/changes/set_default_membership_fee_type.ex @@ -0,0 +1,42 @@ +defmodule Mv.Membership.Member.Changes.SetDefaultMembershipFeeType do + @moduledoc """ + Ash change that automatically assigns the default membership fee type to new members + if no membership_fee_type_id is explicitly provided. + + This change reads the default_membership_fee_type_id from global settings and + assigns it to the member if membership_fee_type_id is nil. + """ + use Ash.Resource.Change + + @spec change(Ash.Changeset.t(), keyword(), Ash.Resource.Change.context()) :: Ash.Changeset.t() + def change(changeset, _opts, _context) do + # Only set default if membership_fee_type_id is not already set + current_type_id = Ash.Changeset.get_attribute(changeset, :membership_fee_type_id) + + if is_nil(current_type_id) do + apply_default_membership_fee_type(changeset) + else + changeset + end + end + + defp apply_default_membership_fee_type(changeset) do + case Mv.Membership.get_settings() do + {:ok, settings} -> + if settings.default_membership_fee_type_id do + Ash.Changeset.force_change_attribute( + changeset, + :membership_fee_type_id, + settings.default_membership_fee_type_id + ) + else + changeset + end + + {:error, _error} -> + # If settings can't be loaded, continue without default + # This prevents member creation from failing if settings are misconfigured + changeset + end + end +end diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index f5a708b..4917c7c 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -21,6 +21,9 @@ defmodule Mv.Membership do use Ash.Domain, extensions: [AshAdmin.Domain, AshPhoenix] + require Ash.Query + import Ash.Expr + admin do show? true end @@ -125,6 +128,29 @@ defmodule Mv.Membership do |> Ash.update(domain: __MODULE__) end + @doc """ + Lists only required custom fields. + + This is an optimized version that filters at the database level instead of + loading all custom fields and filtering in memory. + + ## Returns + + - `{:ok, required_custom_fields}` - List of required custom fields + - `{:error, error}` - Error reading custom fields + + ## Examples + + iex> {:ok, required_fields} = Mv.Membership.list_required_custom_fields() + iex> Enum.all?(required_fields, & &1.required) + true + """ + def list_required_custom_fields do + Mv.Membership.CustomField + |> Ash.Query.filter(expr(required == true)) + |> Ash.read(domain: __MODULE__) + end + @doc """ Updates the member field visibility configuration. diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index 52c0328..eedc47c 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -4,13 +4,15 @@ defmodule Mv.Membership.Setting do ## Overview Settings is a singleton resource that stores global configuration for the association, - such as the club name and branding information. There should only ever be one settings - record in the database. + such as the club name, branding information, and membership fee settings. There should + only ever be one settings record in the database. ## Attributes - `club_name` - The name of the association/club (required, cannot be empty) - `member_field_visibility` - JSONB map storing visibility configuration for member fields (e.g., `%{"street" => false, "house_number" => false}`). Fields not in the map default to `true`. + - `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) ## Singleton Pattern This resource uses a singleton pattern - there should only be one settings record. @@ -22,6 +24,12 @@ defmodule Mv.Membership.Setting do If set, the environment variable value is used as a fallback when no database value exists. Database values always take precedence over environment variables. + ## Membership Fee Settings + - `include_joining_cycle`: When true, members pay from their joining cycle. When false, + they pay from the next full cycle after joining. + - `default_membership_fee_type_id`: The membership fee type automatically assigned to + new members. Can be nil if no default is set. + ## Examples # Get current settings @@ -33,6 +41,9 @@ defmodule Mv.Membership.Setting do # Update member field visibility {:ok, updated} = Mv.Membership.update_member_field_visibility(settings, %{"street" => false, "house_number" => false}) + + # Update membership fee settings + {:ok, updated} = Mv.Membership.update_settings(settings, %{include_joining_cycle: false}) """ use Ash.Resource, domain: Mv.Membership, @@ -54,13 +65,24 @@ defmodule Mv.Membership.Setting do # Used only as fallback in get_settings/0 if settings don't exist # Settings should normally be created via seed script create :create do - accept [:club_name, :member_field_visibility] + accept [ + :club_name, + :member_field_visibility, + :include_joining_cycle, + :default_membership_fee_type_id + ] end update :update do primary? true require_atomic? false - accept [:club_name, :member_field_visibility] + + accept [ + :club_name, + :member_field_visibility, + :include_joining_cycle, + :default_membership_fee_type_id + ] end update :update_member_field_visibility do @@ -68,6 +90,14 @@ defmodule Mv.Membership.Setting do require_atomic? false accept [:member_field_visibility] end + + update :update_membership_fee_settings do + description "Updates the membership fee configuration" + require_atomic? false + accept [:include_joining_cycle, :default_membership_fee_type_id] + + change Mv.Membership.Setting.Changes.NormalizeDefaultFeeTypeId + end end validations do @@ -113,6 +143,41 @@ defmodule Mv.Membership.Setting do end end, on: [:create, :update] + + # Validate default_membership_fee_type_id exists if set + validate fn changeset, _context -> + fee_type_id = + Ash.Changeset.get_attribute(changeset, :default_membership_fee_type_id) + + if fee_type_id do + case Ash.get(Mv.MembershipFees.MembershipFeeType, fee_type_id) do + {:ok, _} -> + :ok + + {:error, %Ash.Error.Invalid{errors: [%Ash.Error.Query.NotFound{} | _]}} -> + {:error, + field: :default_membership_fee_type_id, + message: "Membership fee type not found"} + + {:error, err} -> + # Log unexpected errors (DB timeout, connection errors, etc.) + require Logger + + Logger.warning( + "Unexpected error when validating default_membership_fee_type_id: #{inspect(err)}" + ) + + # Return generic error to user + {:error, + field: :default_membership_fee_type_id, + message: "Could not validate membership fee type"} + end + else + # Optional, can be nil + :ok + end + end, + on: [:create, :update] end attributes do @@ -133,6 +198,26 @@ defmodule Mv.Membership.Setting do description: "Configuration for member field visibility in overview (JSONB map). Keys are member field names (atoms), values are booleans." + # Membership fee settings + attribute :include_joining_cycle, :boolean do + allow_nil? false + default true + public? true + description "Whether to include the joining cycle in membership fee generation" + end + + attribute :default_membership_fee_type_id, :uuid do + allow_nil? true + public? true + description "Default membership fee type ID for new members" + end + timestamps() end + + relationships do + # Optional relationship to the default membership fee type + # Note: We use manual FK (default_membership_fee_type_id attribute) instead of belongs_to + # to avoid circular dependency between Membership and MembershipFees domains + end end diff --git a/lib/membership/setting/changes/normalize_default_fee_type_id.ex b/lib/membership/setting/changes/normalize_default_fee_type_id.ex new file mode 100644 index 0000000..fdbe1c8 --- /dev/null +++ b/lib/membership/setting/changes/normalize_default_fee_type_id.ex @@ -0,0 +1,19 @@ +defmodule Mv.Membership.Setting.Changes.NormalizeDefaultFeeTypeId do + @moduledoc """ + Ash change that normalizes empty strings to nil for default_membership_fee_type_id. + + HTML forms submit empty select values as empty strings (""), but the database + expects nil for optional UUID fields. This change converts "" to nil. + """ + use Ash.Resource.Change + + def change(changeset, _opts, _context) do + default_fee_type_id = Ash.Changeset.get_attribute(changeset, :default_membership_fee_type_id) + + if default_fee_type_id == "" do + Ash.Changeset.force_change_attribute(changeset, :default_membership_fee_type_id, nil) + else + changeset + end + 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 new file mode 100644 index 0000000..a2e1ad0 --- /dev/null +++ b/lib/membership_fees/changes/set_membership_fee_start_date.ex @@ -0,0 +1,174 @@ +defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do + @moduledoc """ + Ash change module that automatically calculates and sets the membership_fee_start_date. + + ## Logic + + 1. Only executes if `membership_fee_start_date` is not manually set + 2. Requires both `join_date` and `membership_fee_type_id` to be present + 3. Reads `include_joining_cycle` setting from global Settings + 4. Reads `interval` from the assigned `membership_fee_type` + 5. Calculates the start date: + - If `include_joining_cycle = true`: First day of the joining cycle + - If `include_joining_cycle = false`: First day of the next cycle after joining + + ## Usage + + In a Member action: + + create :create_member do + # ... + change Mv.MembershipFees.Changes.SetMembershipFeeStartDate + end + + The change module handles all prerequisite checks internally (join_date, membership_fee_type_id). + If any required data is missing, the changeset is returned unchanged with a warning logged. + """ + use Ash.Resource.Change + + require Logger + + alias Mv.MembershipFees.CalendarCycles + + @impl true + def change(changeset, _opts, _context) do + # Only calculate if membership_fee_start_date is not already set + if has_start_date?(changeset) do + changeset + else + calculate_and_set_start_date(changeset) + end + end + + # Check if membership_fee_start_date is already set (either in changeset or data) + defp has_start_date?(changeset) do + # Check if it's being set in this changeset + case Ash.Changeset.fetch_change(changeset, :membership_fee_start_date) do + {:ok, date} when not is_nil(date) -> + true + + _ -> + # Check if it already exists in the data (for updates) + case changeset.data do + %{membership_fee_start_date: date} when not is_nil(date) -> true + _ -> false + end + end + end + + defp calculate_and_set_start_date(changeset) do + with {:ok, join_date} <- get_join_date(changeset), + {:ok, membership_fee_type_id} <- get_membership_fee_type_id(changeset), + {:ok, interval} <- get_interval(membership_fee_type_id), + {:ok, include_joining_cycle} <- get_include_joining_cycle() do + start_date = calculate_start_date(join_date, interval, include_joining_cycle) + Ash.Changeset.force_change_attribute(changeset, :membership_fee_start_date, start_date) + else + {:error, :join_date_not_set} -> + # Missing join_date is expected for partial creates + changeset + + {:error, :membership_fee_type_not_set} -> + # Missing membership_fee_type_id is expected for partial creates + changeset + + {:error, :membership_fee_type_not_found} -> + # This is a data integrity error - membership_fee_type_id references non-existent type + # Return changeset error to fail the action + Ash.Changeset.add_error( + changeset, + 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 + + defp get_join_date(changeset) do + # First check the changeset for changes + case Ash.Changeset.fetch_change(changeset, :join_date) do + {:ok, date} when not is_nil(date) -> + {:ok, date} + + _ -> + # Then check existing data + case changeset.data do + %{join_date: date} when not is_nil(date) -> {:ok, date} + _ -> {:error, :join_date_not_set} + end + end + end + + defp get_membership_fee_type_id(changeset) do + # First check the changeset for changes + case Ash.Changeset.fetch_change(changeset, :membership_fee_type_id) do + {:ok, id} when not is_nil(id) -> + {:ok, id} + + _ -> + # Then check existing data + case changeset.data do + %{membership_fee_type_id: id} when not is_nil(id) -> {:ok, id} + _ -> {:error, :membership_fee_type_not_set} + end + end + end + + defp get_interval(membership_fee_type_id) do + case Ash.get(Mv.MembershipFees.MembershipFeeType, membership_fee_type_id) do + {:ok, %{interval: interval}} -> {:ok, interval} + {:error, _} -> {:error, :membership_fee_type_not_found} + end + end + + defp get_include_joining_cycle do + case Mv.Membership.get_settings() do + {:ok, %{include_joining_cycle: include}} -> {:ok, include} + {:error, _} -> {:ok, true} + end + end + + @doc """ + Calculates the membership fee start date based on join date, interval, and settings. + + ## Parameters + + - `join_date` - The date the member joined + - `interval` - The billing interval (:monthly, :quarterly, :half_yearly, :yearly) + - `include_joining_cycle` - Whether to include the joining cycle + + ## Returns + + The calculated start date (first day of the appropriate cycle). + + ## Examples + + iex> calculate_start_date(~D[2024-03-15], :yearly, true) + ~D[2024-01-01] + + iex> calculate_start_date(~D[2024-03-15], :yearly, false) + ~D[2025-01-01] + + iex> calculate_start_date(~D[2024-03-15], :quarterly, true) + ~D[2024-01-01] + + iex> calculate_start_date(~D[2024-03-15], :quarterly, false) + ~D[2024-04-01] + + """ + @spec calculate_start_date(Date.t(), CalendarCycles.interval(), boolean()) :: Date.t() + def calculate_start_date(join_date, interval, include_joining_cycle) do + if include_joining_cycle do + # Start date is the first day of the joining cycle + CalendarCycles.calculate_cycle_start(join_date, interval) + else + # Start date is the first day of the next cycle after joining + join_cycle_start = CalendarCycles.calculate_cycle_start(join_date, interval) + CalendarCycles.next_cycle_start(join_cycle_start, interval) + end + end +end diff --git a/lib/membership_fees/changes/validate_same_interval.ex b/lib/membership_fees/changes/validate_same_interval.ex new file mode 100644 index 0000000..8c1efb4 --- /dev/null +++ b/lib/membership_fees/changes/validate_same_interval.ex @@ -0,0 +1,148 @@ +defmodule Mv.MembershipFees.Changes.ValidateSameInterval do + @moduledoc """ + Validates that membership fee type changes only allow same-interval types. + + Prevents changing from yearly to monthly, etc. (MVP constraint). + + ## Usage + + In a Member action: + + update :update_member do + # ... + change Mv.MembershipFees.Changes.ValidateSameInterval + end + + The change module only executes when `membership_fee_type_id` is being changed. + If the new type has a different interval than the current type, a validation error is returned. + """ + use Ash.Resource.Change + + @impl true + def change(changeset, _opts, _context) do + if changing_membership_fee_type?(changeset) do + validate_interval_match(changeset) + else + changeset + end + end + + # Check if membership_fee_type_id is being changed + defp changing_membership_fee_type?(changeset) do + Ash.Changeset.changing_attribute?(changeset, :membership_fee_type_id) + end + + # Validate that the new type has the same interval as the current type + defp validate_interval_match(changeset) do + current_type_id = get_current_type_id(changeset) + new_type_id = get_new_type_id(changeset) + + cond do + # If no current type, allow any change (first assignment) + is_nil(current_type_id) -> + changeset + + # If new type is nil, reject the change (membership_fee_type_id is required) + is_nil(new_type_id) -> + add_nil_type_error(changeset) + + # Both types exist - validate intervals match + true -> + validate_intervals_match(changeset, current_type_id, new_type_id) + end + end + + # Validates that intervals match when both types exist + defp validate_intervals_match(changeset, current_type_id, new_type_id) do + case get_intervals(current_type_id, new_type_id) do + {:ok, current_interval, new_interval} -> + if current_interval == new_interval do + changeset + else + add_interval_mismatch_error(changeset, current_interval, new_interval) + end + + {:error, reason} -> + # Fail closed: If we can't load the types, reject the change + # This prevents inconsistent data states + add_type_validation_error(changeset, reason) + end + end + + # Get current type ID from changeset data + defp get_current_type_id(changeset) do + case changeset.data do + %{membership_fee_type_id: type_id} -> type_id + _ -> nil + end + end + + # Get new type ID from changeset + defp get_new_type_id(changeset) do + case Ash.Changeset.fetch_change(changeset, :membership_fee_type_id) do + {:ok, type_id} -> type_id + :error -> nil + end + end + + # Get intervals for both types + defp get_intervals(current_type_id, new_type_id) do + alias Mv.MembershipFees.MembershipFeeType + + case {Ash.get(MembershipFeeType, current_type_id), Ash.get(MembershipFeeType, new_type_id)} do + {{:ok, current_type}, {:ok, new_type}} -> + {:ok, current_type.interval, new_type.interval} + + _ -> + {:error, :type_not_found} + end + end + + # Add validation error for interval mismatch + defp add_interval_mismatch_error(changeset, current_interval, new_interval) do + current_interval_name = format_interval(current_interval) + new_interval_name = format_interval(new_interval) + + message = + "Cannot change membership fee type: current type uses #{current_interval_name} interval, " <> + "new type uses #{new_interval_name} interval. Only same-interval changes are allowed." + + Ash.Changeset.add_error( + changeset, + field: :membership_fee_type_id, + message: message + ) + end + + # Add validation error when types cannot be loaded + defp add_type_validation_error(changeset, _reason) do + message = + "Could not validate membership fee type intervals. " <> + "The current or new membership fee type no longer exists. " <> + "This may indicate a data consistency issue." + + Ash.Changeset.add_error( + changeset, + field: :membership_fee_type_id, + message: message + ) + end + + # Add validation error when trying to set membership_fee_type_id to nil + defp add_nil_type_error(changeset) do + message = "Cannot remove membership fee type. A membership fee type is required." + + Ash.Changeset.add_error( + changeset, + field: :membership_fee_type_id, + message: message + ) + end + + # Format interval atom to human-readable string + defp format_interval(:monthly), do: "monthly" + defp format_interval(:quarterly), do: "quarterly" + defp format_interval(:half_yearly), do: "half-yearly" + defp format_interval(:yearly), do: "yearly" + defp format_interval(interval), do: to_string(interval) +end diff --git a/lib/membership_fees/membership_fee_cycle.ex b/lib/membership_fees/membership_fee_cycle.ex new file mode 100644 index 0000000..4d9c8b7 --- /dev/null +++ b/lib/membership_fees/membership_fee_cycle.ex @@ -0,0 +1,132 @@ +defmodule Mv.MembershipFees.MembershipFeeCycle do + @moduledoc """ + Ash resource representing an individual membership fee cycle for a member. + + ## Overview + MembershipFeeCycle represents a single billing cycle for a member. Each cycle + tracks the payment status and amount for a specific time period. + + ## Attributes + - `cycle_start` - Start date of the billing cycle (aligned to calendar boundaries) + - `amount` - The fee amount for this cycle (stored for audit trail) + - `status` - Payment status: unpaid, paid, or suspended + - `notes` - Optional notes for this cycle + + ## Design Decisions + - **No cycle_end field**: Calculated from cycle_start + interval (from fee type) + - **Amount stored per cycle**: Preserves historical amounts when fee type changes + - **Calendar-aligned cycles**: All cycles start on calendar boundaries + + ## Relationships + - `belongs_to :member` - The member this cycle belongs to + - `belongs_to :membership_fee_type` - The fee type for this cycle + + ## Constraints + - Unique constraint on (member_id, cycle_start) - one cycle per period per member + - CASCADE delete when member is deleted + - RESTRICT delete on membership_fee_type if cycles exist + """ + use Ash.Resource, + domain: Mv.MembershipFees, + data_layer: AshPostgres.DataLayer + + postgres do + table "membership_fee_cycles" + repo Mv.Repo + end + + resource do + description "Individual membership fee cycle for a member" + end + + actions do + defaults [:read, :destroy] + + create :create do + primary? true + accept [:cycle_start, :amount, :status, :notes, :member_id, :membership_fee_type_id] + end + + update :update do + primary? true + accept [:status, :notes, :amount] + end + + update :mark_as_paid do + description "Mark cycle as paid" + require_atomic? false + accept [:notes] + + change fn changeset, _context -> + Ash.Changeset.force_change_attribute(changeset, :status, :paid) + end + end + + update :mark_as_suspended do + description "Mark cycle as suspended" + require_atomic? false + accept [:notes] + + change fn changeset, _context -> + Ash.Changeset.force_change_attribute(changeset, :status, :suspended) + end + end + + update :mark_as_unpaid do + description "Mark cycle as unpaid (for error correction)" + require_atomic? false + accept [:notes] + + change fn changeset, _context -> + Ash.Changeset.force_change_attribute(changeset, :status, :unpaid) + end + end + end + + attributes do + uuid_v7_primary_key :id + + attribute :cycle_start, :date do + allow_nil? false + public? true + description "Start date of the billing cycle" + end + + attribute :amount, :decimal do + allow_nil? false + public? true + + description "Fee amount for this cycle (stored for audit trail, non-negative, max 2 decimal places)" + + constraints min: 0, scale: 2 + end + + attribute :status, :atom do + allow_nil? false + public? true + default :unpaid + description "Payment status of this cycle" + constraints one_of: [:unpaid, :paid, :suspended] + end + + attribute :notes, :string do + allow_nil? true + public? true + description "Optional notes for this cycle" + end + end + + relationships do + belongs_to :member, Mv.Membership.Member do + allow_nil? false + end + + belongs_to :membership_fee_type, Mv.MembershipFees.MembershipFeeType do + allow_nil? false + end + end + + identities do + identity :unique_cycle_per_member, [:member_id, :cycle_start] + end +end diff --git a/lib/membership_fees/membership_fee_type.ex b/lib/membership_fees/membership_fee_type.ex new file mode 100644 index 0000000..01ae625 --- /dev/null +++ b/lib/membership_fees/membership_fee_type.ex @@ -0,0 +1,190 @@ +defmodule Mv.MembershipFees.MembershipFeeType do + @moduledoc """ + Ash resource representing a membership fee type definition. + + ## Overview + MembershipFeeType defines the different types of membership fees that can be + assigned to members. Each type has a fixed interval (billing cycle) and a + default amount. + + ## Attributes + - `name` - Unique name for the fee type (e.g., "Standard", "Reduced", "Family") + - `amount` - The fee amount in the default currency (decimal) + - `interval` - Billing interval: monthly, quarterly, half_yearly, or yearly + - `description` - Optional description for the fee type + + ## Immutability + The `interval` field is immutable after creation. This prevents complex + migration scenarios when changing billing cycles. To change intervals, + create a new fee type and migrate members. + + ## Relationships + - `has_many :members` - Members assigned to this fee type + - `has_many :membership_fee_cycles` - All cycles using this fee type + """ + use Ash.Resource, + domain: Mv.MembershipFees, + data_layer: AshPostgres.DataLayer + + postgres do + table "membership_fee_types" + repo Mv.Repo + end + + resource do + description "Membership fee type definition with interval and amount" + end + + actions do + defaults [:read] + + create :create do + primary? true + accept [:name, :amount, :interval, :description] + end + + update :update do + primary? true + # require_atomic? false because validation queries (member/cycle counts) are not atomic + # DB constraints serve as the final safeguard if data changes between validation and update + require_atomic? false + # Note: interval is NOT in accept list - it's immutable after creation + accept [:name, :amount, :description] + end + + destroy :destroy do + primary? true + + # require_atomic? false because validation queries (member/cycle/settings counts) are not atomic + # DB constraints serve as the final safeguard if data changes between validation and delete + require_atomic? false + end + end + + validations do + # Prevent interval changes after creation + validate fn changeset, _context -> + if Ash.Changeset.changing_attribute?(changeset, :interval) do + case changeset.data do + # Creating new resource, interval can be set + nil -> + :ok + + _existing -> + {:error, + field: :interval, message: "Interval cannot be changed after creation"} + end + else + :ok + end + end, + on: [:update] + + # Prevent deletion if assigned to members + validate fn changeset, _context -> + if changeset.action_type == :destroy do + require Ash.Query + + member_count = + Mv.Membership.Member + |> Ash.Query.filter(membership_fee_type_id == ^changeset.data.id) + |> Ash.count!() + + if member_count > 0 do + {:error, + message: + "Cannot delete membership fee type: #{member_count} member(s) are assigned to it"} + else + :ok + end + else + :ok + end + end, + on: [:destroy] + + # Prevent deletion if cycles exist + validate fn changeset, _context -> + if changeset.action_type == :destroy do + require Ash.Query + + cycle_count = + Mv.MembershipFees.MembershipFeeCycle + |> Ash.Query.filter(membership_fee_type_id == ^changeset.data.id) + |> Ash.count!() + + if cycle_count > 0 do + {:error, + message: + "Cannot delete membership fee type: #{cycle_count} cycle(s) reference it"} + else + :ok + end + else + :ok + end + end, + on: [:destroy] + + # Prevent deletion if used as default in settings + validate fn changeset, _context -> + if changeset.action_type == :destroy do + require Ash.Query + + setting_count = + Mv.Membership.Setting + |> Ash.Query.filter(default_membership_fee_type_id == ^changeset.data.id) + |> Ash.count!() + + if setting_count > 0 do + {:error, + message: "Cannot delete membership fee type: it's used as default in settings"} + else + :ok + end + else + :ok + end + end, + on: [:destroy] + end + + attributes do + uuid_v7_primary_key :id + + attribute :name, :string do + allow_nil? false + public? true + description "Unique name for the membership fee type" + end + + attribute :amount, :decimal do + allow_nil? false + public? true + description "Fee amount in default currency (non-negative, max 2 decimal places)" + constraints min: 0, scale: 2 + end + + attribute :interval, :atom do + allow_nil? false + public? true + description "Billing interval (immutable after creation)" + constraints one_of: [:monthly, :quarterly, :half_yearly, :yearly] + end + + attribute :description, :string do + allow_nil? true + public? true + description "Optional description for the fee type" + end + end + + relationships do + has_many :membership_fee_cycles, Mv.MembershipFees.MembershipFeeCycle + has_many :members, Mv.Membership.Member + end + + identities do + identity :unique_name, [:name] + end +end diff --git a/lib/membership_fees/membership_fees.ex b/lib/membership_fees/membership_fees.ex new file mode 100644 index 0000000..7a2833a --- /dev/null +++ b/lib/membership_fees/membership_fees.ex @@ -0,0 +1,42 @@ +defmodule Mv.MembershipFees do + @moduledoc """ + Ash Domain for membership fee management. + + ## Resources + - `MembershipFeeType` - Defines membership fee types with intervals and amounts + - `MembershipFeeCycle` - Individual membership fee cycles per member + + ## Overview + This domain handles the complete membership fee lifecycle including: + - Fee type definitions (monthly, quarterly, half-yearly, yearly) + - Individual fee cycles for each member + - Payment status tracking (unpaid, paid, suspended) + + ## Architecture Decisions + - `interval` field on MembershipFeeType is immutable after creation + - `cycle_end` is calculated, not stored (from cycle_start + interval) + - `amount` is stored per cycle for audit trail when prices change + """ + use Ash.Domain, + extensions: [AshAdmin.Domain, AshPhoenix] + + admin do + show? true + end + + resources do + resource Mv.MembershipFees.MembershipFeeType do + define :create_membership_fee_type, action: :create + define :list_membership_fee_types, action: :read + define :update_membership_fee_type, action: :update + define :destroy_membership_fee_type, action: :destroy + end + + resource Mv.MembershipFees.MembershipFeeCycle do + define :create_membership_fee_cycle, action: :create + define :list_membership_fee_cycles, action: :read + define :update_membership_fee_cycle, action: :update + define :destroy_membership_fee_cycle, action: :destroy + end + end +end diff --git a/lib/mv/application.ex b/lib/mv/application.ex index b77107e..09eefce 100644 --- a/lib/mv/application.ex +++ b/lib/mv/application.ex @@ -10,6 +10,7 @@ defmodule Mv.Application do children = [ MvWeb.Telemetry, Mv.Repo, + {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}, diff --git a/lib/mv/authorization/authorization.ex b/lib/mv/authorization/authorization.ex new file mode 100644 index 0000000..aac07a9 --- /dev/null +++ b/lib/mv/authorization/authorization.ex @@ -0,0 +1,31 @@ +defmodule Mv.Authorization do + @moduledoc """ + Ash Domain for authorization and role management. + + ## Resources + - `Role` - User roles that reference permission sets + + ## Public API + The domain exposes these main actions: + - Role CRUD: `create_role/1`, `list_roles/0`, `update_role/2`, `destroy_role/1` + + ## Admin Interface + The domain is configured with AshAdmin for management UI. + """ + use Ash.Domain, + extensions: [AshAdmin.Domain, AshPhoenix] + + admin do + show? true + end + + resources do + resource Mv.Authorization.Role do + define :create_role, action: :create_role + define :list_roles, action: :read + define :get_role, action: :read, get_by: [:id] + define :update_role, action: :update_role + define :destroy_role, action: :destroy + end + end +end diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex new file mode 100644 index 0000000..11ddb5a --- /dev/null +++ b/lib/mv/authorization/permission_sets.ex @@ -0,0 +1,294 @@ +defmodule Mv.Authorization.PermissionSets do + @moduledoc """ + Defines the four hardcoded permission sets for the application. + + Each permission set specifies: + - Resource permissions (what CRUD operations on which resources) + - Page permissions (which LiveView pages can be accessed) + - Scopes (own, linked, all) + + ## Permission Sets + + 1. **own_data** - Default for "Mitglied" role + - Can only access own user data and linked member/custom field values + - Cannot create new members or manage system + + 2. **read_only** - For "Vorstand" and "Buchhaltung" roles + - Can read all member data + - Cannot create, update, or delete + + 3. **normal_user** - For "Kassenwart" role + - Create/Read/Update members (no delete for safety), full CRUD on custom field values + - Cannot manage custom fields or users + + 4. **admin** - For "Admin" role + - Unrestricted access to all resources + - Can manage users, roles, custom fields + + ## Usage + + # Get permissions for a role's permission set + permissions = PermissionSets.get_permissions(:admin) + + # Check if a permission set name is valid + PermissionSets.valid_permission_set?("read_only") # => true + + # Convert string to atom safely + {:ok, atom} = PermissionSets.permission_set_name_to_atom("own_data") + + ## Performance + + All functions are pure and intended to be constant-time. Permission lookups + are very fast (typically < 1 microsecond in practice) as they are simple + pattern matches and map lookups with no database queries or external calls. + """ + + @type scope :: :own | :linked | :all + @type action :: :read | :create | :update | :destroy + + @type resource_permission :: %{ + resource: String.t(), + action: action(), + scope: scope(), + granted: boolean() + } + + @type permission_set :: %{ + resources: [resource_permission()], + pages: [String.t()] + } + + @doc """ + Returns the list of all valid permission set names. + + ## Examples + + iex> PermissionSets.all_permission_sets() + [:own_data, :read_only, :normal_user, :admin] + """ + @spec all_permission_sets() :: [atom()] + def all_permission_sets do + [:own_data, :read_only, :normal_user, :admin] + end + + @doc """ + Returns permissions for the given permission set. + + ## Examples + + iex> permissions = PermissionSets.get_permissions(:admin) + iex> Enum.any?(permissions.resources, fn p -> + ...> p.resource == "User" and p.action == :destroy + ...> end) + true + + iex> PermissionSets.get_permissions(:invalid) + ** (ArgumentError) invalid permission set: :invalid. Must be one of: [:own_data, :read_only, :normal_user, :admin] + """ + @spec get_permissions(atom()) :: permission_set() + + def get_permissions(set) when set not in [:own_data, :read_only, :normal_user, :admin] do + raise ArgumentError, + "invalid permission set: #{inspect(set)}. Must be one of: #{inspect(all_permission_sets())}" + end + + def get_permissions(:own_data) do + %{ + resources: [ + # User: Can always read/update own credentials + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Can read/update linked member + %{resource: "Member", action: :read, scope: :linked, granted: true}, + %{resource: "Member", action: :update, scope: :linked, granted: true}, + + # CustomFieldValue: Can read/update custom field values of linked member + %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, + + # CustomField: Can read all (needed for forms) + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + # Home page + "/", + # Own profile + "/profile", + # Linked member detail (filtered by policy) + "/members/:id" + ] + } + end + + def get_permissions(:read_only) do + %{ + resources: [ + # User: Can read/update own credentials only + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Can read all members, no modifications + %{resource: "Member", action: :read, scope: :all, granted: true}, + + # CustomFieldValue: Can read all custom field values + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + + # CustomField: Can read all + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + "/", + # Own profile + "/profile", + # Member list + "/members", + # Member detail + "/members/:id", + # Custom field values overview + "/custom_field_values", + # Custom field value detail + "/custom_field_values/:id" + ] + } + end + + def get_permissions(:normal_user) do + %{ + resources: [ + # User: Can read/update own credentials only + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Full CRUD except destroy (safety) + %{resource: "Member", action: :read, scope: :all, granted: true}, + %{resource: "Member", action: :create, scope: :all, granted: true}, + %{resource: "Member", action: :update, scope: :all, granted: true}, + # Note: destroy intentionally omitted for safety + + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, + + # CustomField: Read only (admin manages definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + "/", + # Own profile + "/profile", + "/members", + # Create member + "/members/new", + "/members/:id", + # Edit member + "/members/:id/edit", + "/custom_field_values", + # Custom field value detail + "/custom_field_values/:id", + "/custom_field_values/new", + "/custom_field_values/:id/edit" + ] + } + end + + def get_permissions(:admin) do + %{ + resources: [ + # User: Full management including other users + %{resource: "User", action: :read, scope: :all, granted: true}, + %{resource: "User", action: :create, scope: :all, granted: true}, + %{resource: "User", action: :update, scope: :all, granted: true}, + %{resource: "User", action: :destroy, scope: :all, granted: true}, + + # Member: Full CRUD + %{resource: "Member", action: :read, scope: :all, granted: true}, + %{resource: "Member", action: :create, scope: :all, granted: true}, + %{resource: "Member", action: :update, scope: :all, granted: true}, + %{resource: "Member", action: :destroy, scope: :all, granted: true}, + + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, + + # CustomField: Full CRUD (admin manages custom field definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true}, + %{resource: "CustomField", action: :create, scope: :all, granted: true}, + %{resource: "CustomField", action: :update, scope: :all, granted: true}, + %{resource: "CustomField", action: :destroy, scope: :all, granted: true}, + + # Role: Full CRUD (admin manages roles) + %{resource: "Role", action: :read, scope: :all, granted: true}, + %{resource: "Role", action: :create, scope: :all, granted: true}, + %{resource: "Role", action: :update, scope: :all, granted: true}, + %{resource: "Role", action: :destroy, scope: :all, granted: true} + ], + pages: [ + # Wildcard: Admin can access all pages + "*" + ] + } + end + + def get_permissions(invalid) do + raise ArgumentError, + "invalid permission set: #{inspect(invalid)}. Must be one of: #{inspect(all_permission_sets())}" + end + + @doc """ + Checks if a permission set name (string or atom) is valid. + + ## Examples + + iex> PermissionSets.valid_permission_set?("admin") + true + + iex> PermissionSets.valid_permission_set?(:read_only) + true + + iex> PermissionSets.valid_permission_set?("invalid") + false + """ + @spec valid_permission_set?(any()) :: boolean() + def valid_permission_set?(name) when is_binary(name) do + case permission_set_name_to_atom(name) do + {:ok, _atom} -> true + {:error, _} -> false + end + end + + def valid_permission_set?(name) when is_atom(name) do + name in all_permission_sets() + end + + def valid_permission_set?(_), do: false + + @doc """ + Converts a permission set name string to atom safely. + + ## Examples + + iex> PermissionSets.permission_set_name_to_atom("admin") + {:ok, :admin} + + iex> PermissionSets.permission_set_name_to_atom("invalid") + {:error, :invalid_permission_set} + """ + @spec permission_set_name_to_atom(String.t()) :: + {:ok, atom()} | {:error, :invalid_permission_set} + def permission_set_name_to_atom(name) when is_binary(name) do + atom = String.to_existing_atom(name) + + if valid_permission_set?(atom) do + {:ok, atom} + else + {:error, :invalid_permission_set} + end + rescue + ArgumentError -> {:error, :invalid_permission_set} + end +end diff --git a/lib/mv/authorization/role.ex b/lib/mv/authorization/role.ex new file mode 100644 index 0000000..da43510 --- /dev/null +++ b/lib/mv/authorization/role.ex @@ -0,0 +1,142 @@ +defmodule Mv.Authorization.Role do + @moduledoc """ + Represents a user role that references a permission set. + + Roles are stored in the database and link users to permission sets. + Each role has a `permission_set_name` that references one of the four + hardcoded permission sets defined in `Mv.Authorization.PermissionSets`. + + ## Fields + + - `name` - Unique role name (e.g., "Vorstand", "Admin") + - `description` - Human-readable description of the role + - `permission_set_name` - Must be one of: "own_data", "read_only", "normal_user", "admin" + - `is_system_role` - If true, role cannot be deleted (protects critical roles like "Mitglied") + + ## Relationships + + - `has_many :users` - Users assigned to this role + + ## Validations + + - `permission_set_name` must be a valid permission set (checked against PermissionSets.all_permission_sets/0) + - `name` must be unique + - System roles cannot be deleted (enforced via validation) + + ## Examples + + # Create a new role + {:ok, role} = Mv.Authorization.create_role(%{ + name: "Vorstand", + description: "Board member with read access", + permission_set_name: "read_only" + }) + + # List all roles + {:ok, roles} = Mv.Authorization.list_roles() + """ + use Ash.Resource, + domain: Mv.Authorization, + data_layer: AshPostgres.DataLayer + + 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 + define :create_role + define :list_roles, action: :read + define :update_role + define :destroy_role, action: :destroy + end + + actions do + defaults [:read] + + create :create_role do + primary? true + # is_system_role is intentionally excluded - should only be set via seeds/internal actions + accept [:name, :description, :permission_set_name] + # Note: In Ash 3.0, require_atomic? is not available for create actions + # Custom validations will still work + end + + update :update_role do + primary? true + # is_system_role is intentionally excluded - should only be set via seeds/internal actions + accept [:name, :description, :permission_set_name] + # Required because custom validation functions cannot be executed atomically + require_atomic? false + end + + destroy :destroy do + # Required because custom validation functions cannot be executed atomically + require_atomic? false + end + end + + validations do + validate one_of( + :permission_set_name, + Mv.Authorization.PermissionSets.all_permission_sets() + |> Enum.map(&Atom.to_string/1) + ), + message: + "must be one of: #{Mv.Authorization.PermissionSets.all_permission_sets() |> Enum.map_join(", ", &Atom.to_string/1)}" + + validate fn changeset, _context -> + if changeset.data.is_system_role do + {:error, + field: :is_system_role, + message: + "Cannot delete system role. System roles are required for the application to function."} + else + :ok + end + end, + on: [:destroy] + end + + attributes do + uuid_v7_primary_key :id + + attribute :name, :string do + allow_nil? false + public? true + end + + attribute :description, :string do + allow_nil? true + public? true + end + + attribute :permission_set_name, :string do + allow_nil? false + public? true + end + + attribute :is_system_role, :boolean do + allow_nil? false + default false + public? true + end + + timestamps() + end + + relationships do + has_many :users, Mv.Accounts.User do + destination_attribute :role_id + end + end + + identities do + identity :unique_name, [:name] + end +end diff --git a/lib/mv/config.ex b/lib/mv/config.ex new file mode 100644 index 0000000..5e6ba90 --- /dev/null +++ b/lib/mv/config.ex @@ -0,0 +1,24 @@ +defmodule Mv.Config do + @moduledoc """ + Configuration helper functions for the application. + + Provides centralized access to configuration values to avoid + magic strings/atoms scattered throughout the codebase. + """ + + @doc """ + Returns whether SQL sandbox mode is enabled. + + SQL sandbox mode is typically enabled in test environments + to allow concurrent database access in tests. + + ## Returns + + - `true` if SQL sandbox is enabled + - `false` otherwise + """ + @spec sql_sandbox?() :: boolean() + def sql_sandbox? do + Application.get_env(:mv, :sql_sandbox, false) + end +end diff --git a/lib/mv/constants.ex b/lib/mv/constants.ex index 7bfb07b..c81dbd6 100644 --- a/lib/mv/constants.ex +++ b/lib/mv/constants.ex @@ -7,7 +7,6 @@ defmodule Mv.Constants do :first_name, :last_name, :email, - :paid, :phone_number, :join_date, :exit_date, @@ -15,7 +14,8 @@ defmodule Mv.Constants do :city, :street, :house_number, - :postal_code + :postal_code, + :membership_fee_start_date ] @custom_field_prefix "custom_field_" diff --git a/lib/mv/membership_fees/calendar_cycles.ex b/lib/mv/membership_fees/calendar_cycles.ex new file mode 100644 index 0000000..9bc3afa --- /dev/null +++ b/lib/mv/membership_fees/calendar_cycles.ex @@ -0,0 +1,337 @@ +defmodule Mv.MembershipFees.CalendarCycles do + @moduledoc """ + Calendar-based cycle calculation functions for membership fees. + + This module provides functions for calculating cycle boundaries + based on interval types (monthly, quarterly, half-yearly, yearly). + + The calculation functions (`calculate_cycle_start/3`, `calculate_cycle_end/2`, + `next_cycle_start/2`) are pure functions with no side effects. + + The time-dependent functions (`current_cycle?/3`, `last_completed_cycle?/3`) + depend on a date parameter for testability. Their 2-argument variants + (`current_cycle?/2`, `last_completed_cycle?/2`) use `Date.utc_today()` and + are not referentially transparent. + + ## Interval Types + + - `:monthly` - Cycles from 1st to last day of each month + - `:quarterly` - Cycles from 1st of Jan/Apr/Jul/Oct to last day of quarter + - `:half_yearly` - Cycles from 1st of Jan/Jul to last day of half-year + - `:yearly` - Cycles from Jan 1st to Dec 31st + + ## Examples + + iex> date = ~D[2024-03-15] + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_start(date, :monthly) + ~D[2024-03-01] + + iex> cycle_start = ~D[2024-01-01] + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle_start, :yearly) + ~D[2024-12-31] + + iex> cycle_start = ~D[2024-01-01] + iex> Mv.MembershipFees.CalendarCycles.next_cycle_start(cycle_start, :yearly) + ~D[2025-01-01] + """ + + @typedoc """ + Interval type for membership fee cycles. + + - `:monthly` - Monthly cycles (1st to last day of month) + - `:quarterly` - Quarterly cycles (1st of quarter to last day of quarter) + - `:half_yearly` - Half-yearly cycles (1st of half-year to last day of half-year) + - `:yearly` - Yearly cycles (Jan 1st to Dec 31st) + """ + @type interval :: :monthly | :quarterly | :half_yearly | :yearly + + @doc """ + Calculates the start date of the cycle that contains the reference date. + + ## Parameters + + - `date` - Ignored in this 3-argument version (kept for API consistency) + - `interval` - The interval type (`:monthly`, `:quarterly`, `:half_yearly`, `:yearly`) + - `reference_date` - The date used to determine which cycle to calculate + + ## Returns + + The start date of the cycle containing the reference date. + + ## Examples + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_start(~D[2024-03-15], :monthly, ~D[2024-05-20]) + ~D[2024-05-01] + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_start(~D[2024-03-15], :quarterly, ~D[2024-05-20]) + ~D[2024-04-01] + """ + @spec calculate_cycle_start(Date.t(), interval(), Date.t()) :: Date.t() + def calculate_cycle_start(_date, interval, reference_date) do + case interval do + :monthly -> monthly_cycle_start(reference_date) + :quarterly -> quarterly_cycle_start(reference_date) + :half_yearly -> half_yearly_cycle_start(reference_date) + :yearly -> yearly_cycle_start(reference_date) + end + end + + @doc """ + Calculates the start date of the cycle that contains the given date. + + This is a convenience function that calls `calculate_cycle_start/3` with `date` as both + the input and reference date. + + ## Parameters + + - `date` - The date used to determine which cycle to calculate + - `interval` - The interval type (`:monthly`, `:quarterly`, `:half_yearly`, `:yearly`) + + ## Returns + + The start date of the cycle containing the given date. + + ## Examples + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_start(~D[2024-03-15], :monthly) + ~D[2024-03-01] + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_start(~D[2024-05-15], :quarterly) + ~D[2024-04-01] + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_start(~D[2024-09-15], :half_yearly) + ~D[2024-07-01] + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_start(~D[2024-12-15], :yearly) + ~D[2024-01-01] + """ + @spec calculate_cycle_start(Date.t(), interval()) :: Date.t() + def calculate_cycle_start(date, interval) do + calculate_cycle_start(date, interval, date) + end + + @doc """ + Calculates the end date of a cycle based on its start date and interval. + + ## Parameters + + - `cycle_start` - The start date of the cycle + - `interval` - The interval type + + ## Returns + + The end date of the cycle. + + ## Examples + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_end(~D[2024-03-01], :monthly) + ~D[2024-03-31] + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_end(~D[2024-02-01], :monthly) + ~D[2024-02-29] + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_end(~D[2024-01-01], :quarterly) + ~D[2024-03-31] + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_end(~D[2024-01-01], :half_yearly) + ~D[2024-06-30] + + iex> Mv.MembershipFees.CalendarCycles.calculate_cycle_end(~D[2024-01-01], :yearly) + ~D[2024-12-31] + """ + @spec calculate_cycle_end(Date.t(), interval()) :: Date.t() + def calculate_cycle_end(cycle_start, interval) do + case interval do + :monthly -> monthly_cycle_end(cycle_start) + :quarterly -> quarterly_cycle_end(cycle_start) + :half_yearly -> half_yearly_cycle_end(cycle_start) + :yearly -> yearly_cycle_end(cycle_start) + end + end + + @doc """ + Calculates the start date of the next cycle. + + ## Parameters + + - `cycle_start` - The start date of the current cycle + - `interval` - The interval type + + ## Returns + + The start date of the next cycle. + + ## Examples + + iex> Mv.MembershipFees.CalendarCycles.next_cycle_start(~D[2024-01-01], :monthly) + ~D[2024-02-01] + + iex> Mv.MembershipFees.CalendarCycles.next_cycle_start(~D[2024-01-01], :quarterly) + ~D[2024-04-01] + + iex> Mv.MembershipFees.CalendarCycles.next_cycle_start(~D[2024-01-01], :half_yearly) + ~D[2024-07-01] + + iex> Mv.MembershipFees.CalendarCycles.next_cycle_start(~D[2024-01-01], :yearly) + ~D[2025-01-01] + """ + @spec next_cycle_start(Date.t(), interval()) :: Date.t() + def next_cycle_start(cycle_start, interval) do + cycle_end = calculate_cycle_end(cycle_start, interval) + next_date = Date.add(cycle_end, 1) + calculate_cycle_start(next_date, interval) + end + + @doc """ + Checks if the cycle contains the given date. + + ## Parameters + + - `cycle_start` - The start date of the cycle + - `interval` - The interval type + - `today` - The date to check (defaults to today's date) + + ## Returns + + `true` if the given date is within the cycle, `false` otherwise. + + ## Examples + + iex> Mv.MembershipFees.CalendarCycles.current_cycle?(~D[2024-03-01], :monthly, ~D[2024-03-15]) + true + + iex> Mv.MembershipFees.CalendarCycles.current_cycle?(~D[2024-02-01], :monthly, ~D[2024-03-15]) + false + + iex> Mv.MembershipFees.CalendarCycles.current_cycle?(~D[2024-03-01], :monthly, ~D[2024-03-01]) + true + + iex> Mv.MembershipFees.CalendarCycles.current_cycle?(~D[2024-03-01], :monthly, ~D[2024-03-31]) + true + """ + @spec current_cycle?(Date.t(), interval(), Date.t()) :: boolean() + def current_cycle?(cycle_start, interval, today) do + cycle_end = calculate_cycle_end(cycle_start, interval) + + Date.compare(cycle_start, today) in [:lt, :eq] and + Date.compare(today, cycle_end) in [:lt, :eq] + end + + @spec current_cycle?(Date.t(), interval()) :: boolean() + def current_cycle?(cycle_start, interval) do + current_cycle?(cycle_start, interval, Date.utc_today()) + end + + @doc """ + Checks if the cycle is the last completed cycle. + + A cycle is considered the last completed cycle if: + - The cycle has ended (cycle_end < today) + - The next cycle has not ended yet (today <= next_end) + + In other words: `cycle_end < today <= next_end` + + ## Parameters + + - `cycle_start` - The start date of the cycle + - `interval` - The interval type + - `today` - The date to check against (defaults to today's date) + + ## Returns + + `true` if the cycle is the last completed cycle, `false` otherwise. + + ## Examples + + iex> Mv.MembershipFees.CalendarCycles.last_completed_cycle?(~D[2024-03-01], :monthly, ~D[2024-04-01]) + true + + iex> Mv.MembershipFees.CalendarCycles.last_completed_cycle?(~D[2024-03-01], :monthly, ~D[2024-03-15]) + false + + iex> Mv.MembershipFees.CalendarCycles.last_completed_cycle?(~D[2024-02-01], :monthly, ~D[2024-04-15]) + false + """ + @spec last_completed_cycle?(Date.t(), interval(), Date.t()) :: boolean() + def last_completed_cycle?(cycle_start, interval, today) do + cycle_end = calculate_cycle_end(cycle_start, interval) + + # Cycle must have ended (cycle_end < today) + case Date.compare(today, cycle_end) do + :gt -> + # Check if this is the most recent completed cycle + # by verifying that the next cycle hasn't ended yet (today <= next_end) + next_start = next_cycle_start(cycle_start, interval) + next_end = calculate_cycle_end(next_start, interval) + + Date.compare(today, next_end) in [:lt, :eq] + + _ -> + false + end + end + + @spec last_completed_cycle?(Date.t(), interval()) :: boolean() + def last_completed_cycle?(cycle_start, interval) do + last_completed_cycle?(cycle_start, interval, Date.utc_today()) + end + + # Private helper functions + + defp monthly_cycle_start(date) do + Date.new!(date.year, date.month, 1) + end + + defp monthly_cycle_end(cycle_start) do + Date.end_of_month(cycle_start) + end + + defp quarterly_cycle_start(date) do + quarter_start_month = + case date.month do + m when m in [1, 2, 3] -> 1 + m when m in [4, 5, 6] -> 4 + m when m in [7, 8, 9] -> 7 + m when m in [10, 11, 12] -> 10 + end + + Date.new!(date.year, quarter_start_month, 1) + end + + defp quarterly_cycle_end(cycle_start) do + # Ensure cycle_start is aligned to quarter boundary + # This handles cases where cycle_start might not be at the correct quarter start (e.g., month 12) + aligned_start = quarterly_cycle_start(cycle_start) + + case aligned_start.month do + 1 -> Date.new!(aligned_start.year, 3, 31) + 4 -> Date.new!(aligned_start.year, 6, 30) + 7 -> Date.new!(aligned_start.year, 9, 30) + 10 -> Date.new!(aligned_start.year, 12, 31) + end + end + + defp half_yearly_cycle_start(date) do + half_start_month = if date.month in 1..6, do: 1, else: 7 + Date.new!(date.year, half_start_month, 1) + end + + defp half_yearly_cycle_end(cycle_start) do + # Ensure cycle_start is aligned to half-year boundary + # This handles cases where cycle_start might not be at the correct half-year start (e.g., month 10) + aligned_start = half_yearly_cycle_start(cycle_start) + + case aligned_start.month do + 1 -> Date.new!(aligned_start.year, 6, 30) + 7 -> Date.new!(aligned_start.year, 12, 31) + end + end + + defp yearly_cycle_start(date) do + Date.new!(date.year, 1, 1) + end + + defp yearly_cycle_end(cycle_start) do + Date.new!(cycle_start.year, 12, 31) + end +end diff --git a/lib/mv/membership_fees/cycle_generation_job.ex b/lib/mv/membership_fees/cycle_generation_job.ex new file mode 100644 index 0000000..71a3158 --- /dev/null +++ b/lib/mv/membership_fees/cycle_generation_job.ex @@ -0,0 +1,174 @@ +defmodule Mv.MembershipFees.CycleGenerationJob do + @moduledoc """ + Scheduled job for generating membership fee cycles. + + This module provides a skeleton for scheduled cycle generation. + In the future, this can be integrated with Oban or similar job processing libraries. + + ## Current Implementation + + Currently provides manual execution functions that can be called: + - From IEx console for administrative tasks + - From a cron job via a Mix task + - From the admin UI (future) + + ## Future Oban Integration + + When Oban is added to the project, this module can be converted to an Oban worker: + + defmodule Mv.MembershipFees.CycleGenerationJob do + use Oban.Worker, + queue: :membership_fees, + max_attempts: 3 + + @impl Oban.Worker + def perform(%Oban.Job{}) do + Mv.MembershipFees.CycleGenerator.generate_cycles_for_all_members() + end + end + + ## Usage + + # Manual execution from IEx + Mv.MembershipFees.CycleGenerationJob.run() + + # Check if cycles need to be generated + Mv.MembershipFees.CycleGenerationJob.pending_members_count() + + """ + + alias Mv.MembershipFees.CycleGenerator + + require Ash.Query + require Logger + + @doc """ + Runs the cycle generation job for all active members. + + This is the main entry point for scheduled execution. + + ## Returns + + - `{:ok, results}` - Map with success/failed counts + - `{:error, reason}` - Error with reason + + ## Examples + + iex> Mv.MembershipFees.CycleGenerationJob.run() + {:ok, %{success: 45, failed: 0, total: 45}} + + """ + @spec run() :: {:ok, map()} | {:error, term()} + def run do + Logger.info("Starting membership fee cycle generation job") + start_time = System.monotonic_time(:millisecond) + + result = CycleGenerator.generate_cycles_for_all_members() + + elapsed = System.monotonic_time(:millisecond) - start_time + + case result do + {:ok, stats} -> + Logger.info( + "Cycle generation completed in #{elapsed}ms: #{stats.success} success, #{stats.failed} failed, #{stats.total} total" + ) + + result + + {:error, reason} -> + Logger.error("Cycle generation failed: #{inspect(reason)}") + result + end + end + + @doc """ + Runs cycle generation with custom options. + + ## Options + + - `:today` - Override today's date (useful for testing or catch-up) + - `:batch_size` - Number of members to process in parallel + + ## Examples + + # Generate cycles as if today was a specific date + Mv.MembershipFees.CycleGenerationJob.run(today: ~D[2024-12-31]) + + # Process with smaller batch size + Mv.MembershipFees.CycleGenerationJob.run(batch_size: 5) + + """ + @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) + + result = CycleGenerator.generate_cycles_for_all_members(opts) + + elapsed = System.monotonic_time(:millisecond) - start_time + + case result do + {:ok, stats} -> + Logger.info( + "Cycle generation completed in #{elapsed}ms: #{stats.success} success, #{stats.failed} failed, #{stats.total} total" + ) + + result + + {:error, reason} -> + Logger.error("Cycle generation failed: #{inspect(reason)}") + result + end + end + + @doc """ + Returns the count of members that need cycle generation. + + A member needs cycle generation if: + - Has a membership_fee_type assigned + - Has a join_date set + - Is active (no exit_date or exit_date >= today) + + ## Returns + + - `{:ok, count}` - Number of members needing generation + - `{:error, reason}` - Error with reason + + """ + @spec pending_members_count() :: {:ok, non_neg_integer()} | {:error, term()} + def pending_members_count do + today = Date.utc_today() + + query = + Mv.Membership.Member + |> Ash.Query.filter(not is_nil(membership_fee_type_id)) + |> Ash.Query.filter(not is_nil(join_date)) + |> Ash.Query.filter(is_nil(exit_date) or exit_date >= ^today) + + case Ash.count(query) do + {:ok, count} -> {:ok, count} + {:error, reason} -> {:error, reason} + end + end + + @doc """ + Generates cycles for a specific member by ID. + + Useful for administrative tasks or manual corrections. + + ## Parameters + + - `member_id` - The UUID of the member + + ## Returns + + - `{:ok, cycles}` - List of newly created cycles + - `{:error, reason}` - Error with reason + + """ + @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) + end +end diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex new file mode 100644 index 0000000..7d6c798 --- /dev/null +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -0,0 +1,460 @@ +defmodule Mv.MembershipFees.CycleGenerator do + @moduledoc """ + Module for generating membership fee cycles for members. + + This module provides functions to automatically generate membership fee cycles + based on a member's fee type, start date, and exit date. + + ## Algorithm + + 1. Load member with relationships (membership_fee_type, membership_fee_cycles) + 2. Determine the generation start point: + - If NO cycles exist: Start from `membership_fee_start_date` (or calculated from `join_date`) + - If cycles exist: Start from the cycle AFTER the last existing one + 3. Generate all cycle starts from the determined start point to today (or `exit_date`) + 4. Create new cycles with the current amount from `membership_fee_type` + + ## Important: Gap Handling + + **Gaps are NOT filled.** If a cycle was explicitly deleted (e.g., 2023 was deleted + but 2022 and 2024 exist), the generator will NOT recreate the deleted cycle. + It always continues from the LAST existing cycle, regardless of any gaps. + + This behavior ensures that manually deleted cycles remain deleted and prevents + unwanted automatic recreation of intentionally removed cycles. + + ## Concurrency + + Uses PostgreSQL advisory locks to prevent race conditions when generating + cycles for the same member concurrently. + + ## Examples + + # Generate cycles for a single member + {:ok, cycles} = CycleGenerator.generate_cycles_for_member(member) + + # Generate cycles for all active members + {:ok, results} = CycleGenerator.generate_cycles_for_all_members() + + """ + + alias Mv.Membership.Member + alias Mv.MembershipFees.CalendarCycles + alias Mv.MembershipFees.Changes.SetMembershipFeeStartDate + alias Mv.MembershipFees.MembershipFeeCycle + alias Mv.Repo + + require Ash.Query + require Logger + + @type generate_result :: + {:ok, [MembershipFeeCycle.t()], [Ash.Notifier.Notification.t()]} | {:error, term()} + + @doc """ + Generates membership fee cycles for a single member. + + Uses an advisory lock to prevent concurrent generation for the same member. + + ## Parameters + + - `member` - The member struct or member ID + - `opts` - Options: + - `:today` - Override today's date (useful for testing) + + ## Returns + + - `{:ok, cycles, notifications}` - List of newly created cycles and notifications + - `{:error, reason}` - Error with reason + + ## Examples + + {:ok, cycles, notifications} = CycleGenerator.generate_cycles_for_member(member) + {:ok, cycles, notifications} = CycleGenerator.generate_cycles_for_member(member_id) + {:ok, cycles, notifications} = CycleGenerator.generate_cycles_for_member(member, today: ~D[2024-12-31]) + + """ + @spec generate_cycles_for_member(Member.t() | String.t(), keyword()) :: generate_result() + def generate_cycles_for_member(member_or_id, opts \\ []) + + def generate_cycles_for_member(member_id, opts) when is_binary(member_id) do + case load_member(member_id) do + {:ok, member} -> generate_cycles_for_member(member, opts) + {:error, reason} -> {:error, reason} + end + end + + def generate_cycles_for_member(%Member{} = member, opts) do + today = Keyword.get(opts, :today, Date.utc_today()) + skip_lock? = Keyword.get(opts, :skip_lock?, false) + + do_generate_cycles_with_lock(member, today, skip_lock?) + end + + # Generate cycles with lock handling + # Returns {:ok, cycles, notifications} - notifications are never sent here, + # they should be returned to the caller (e.g., via after_action hook) + defp do_generate_cycles_with_lock(member, today, true = _skip_lock?) do + # Lock already set by caller (e.g., regenerate_cycles_on_type_change) + # Just generate cycles without additional locking + do_generate_cycles(member, today) + end + + defp do_generate_cycles_with_lock(member, today, false) do + lock_key = :erlang.phash2(member.id) + + Repo.transaction(fn -> + Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + + case do_generate_cycles(member, today) do + {:ok, cycles, notifications} -> + # Return cycles and notifications - do NOT send notifications here + # They will be sent by the caller (e.g., via after_action hook) + {cycles, notifications} + + {:error, reason} -> + Repo.rollback(reason) + end + end) + |> case do + {:ok, {cycles, notifications}} -> {:ok, cycles, notifications} + {:error, reason} -> {:error, reason} + end + end + + @doc """ + Generates membership fee cycles for all members with a fee type assigned. + + This includes both active and inactive (left) members. Inactive members + will have cycles generated up to their exit_date if they don't have cycles + for that period yet. This allows for catch-up generation of missing cycles. + + Members processed are those who: + - Have a membership_fee_type assigned + - Have a join_date set + + The exit_date boundary is respected during generation (not in the query), + so inactive members will get cycles up to their exit date. + + ## Parameters + + - `opts` - Options: + - `:today` - Override today's date (useful for testing) + - `:batch_size` - Number of members to process in parallel (default: 10) + + ## Returns + + - `{:ok, results}` - Map with :success and :failed counts + - `{:error, reason}` - Error with reason + + """ + @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) + + # Query ALL members with fee type assigned (including inactive/left members) + # The exit_date boundary is applied during cycle generation, not here. + # This allows catch-up generation for members who left but are missing cycles. + query = + Member + |> Ash.Query.filter(not is_nil(membership_fee_type_id)) + |> Ash.Query.filter(not is_nil(join_date)) + + case Ash.read(query) do + {:ok, members} -> + results = process_members_in_batches(members, batch_size, today) + {:ok, build_results_summary(results)} + + {:error, reason} -> + {:error, reason} + end + end + + defp process_members_in_batches(members, batch_size, today) do + members + |> Enum.chunk_every(batch_size) + |> Enum.flat_map(&process_batch(&1, today)) + end + + defp process_batch(batch, today) do + batch + |> Task.async_stream(fn member -> + process_member_cycle_generation(member, today) + end) + |> Enum.map(fn + {:ok, result} -> + result + + {:exit, reason} -> + # Task crashed - log and return error tuple + Logger.error("Task crashed during cycle generation: #{inspect(reason)}") + {nil, {:error, {:task_exit, reason}}} + end) + end + + # Process cycle generation for a single member in batch job + # Returns {member_id, result} tuple where result is {:ok, cycles, notifications} or {:error, reason} + 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) + {member.id, ok} + + {:error, _reason} = err -> + {member.id, err} + end + end + + # Send notifications for batch job + # This is a top-level job, so we need to send notifications explicitly + defp send_notifications_for_batch_job(notifications) do + if Enum.any?(notifications) do + Ash.Notifier.notify(notifications) + end + end + + defp build_results_summary(results) do + success_count = Enum.count(results, fn {_id, result} -> match?({:ok, _, _}, result) end) + failed_count = Enum.count(results, fn {_id, result} -> match?({:error, _}, result) end) + + %{success: success_count, failed: failed_count, total: length(results)} + end + + # Private functions + + defp load_member(member_id) do + Member + |> Ash.Query.filter(id == ^member_id) + |> Ash.Query.load([:membership_fee_type, :membership_fee_cycles]) + |> Ash.read_one() + |> case do + {:ok, nil} -> {:error, :member_not_found} + {:ok, member} -> {:ok, member} + {:error, reason} -> {:error, reason} + end + end + + defp do_generate_cycles(member, today) do + # Reload member with relationships to ensure fresh data + case load_member(member.id) do + {:ok, member} -> + cond do + is_nil(member.membership_fee_type_id) -> + {:error, :no_membership_fee_type} + + is_nil(member.join_date) -> + {:error, :no_join_date} + + true -> + generate_missing_cycles(member, today) + end + + {:error, reason} -> + {:error, reason} + end + end + + defp generate_missing_cycles(member, today) do + fee_type = member.membership_fee_type + interval = fee_type.interval + amount = fee_type.amount + existing_cycles = member.membership_fee_cycles || [] + + # Determine start point based on existing cycles + # Note: We do NOT fill gaps - only generate from the last existing cycle onwards + start_date = determine_generation_start(member, existing_cycles, interval) + + # Determine end date (today or exit_date, whichever is earlier) + end_date = determine_end_date(member, today) + + # Only generate if start_date <= end_date + if start_date && Date.compare(start_date, end_date) != :gt do + cycle_starts = generate_cycle_starts(start_date, end_date, interval) + create_cycles(cycle_starts, member.id, fee_type.id, amount) + else + {:ok, [], []} + end + end + + # No existing cycles: start from membership_fee_start_date + defp determine_generation_start(member, [], interval) do + determine_start_date(member, interval) + end + + # Has existing cycles: start from the cycle AFTER the last one + # This ensures gaps (deleted cycles) are NOT filled + defp determine_generation_start(_member, existing_cycles, interval) do + last_cycle_start = + existing_cycles + |> Enum.map(& &1.cycle_start) + |> Enum.max(Date) + + CalendarCycles.next_cycle_start(last_cycle_start, interval) + end + + defp determine_start_date(member, interval) do + if member.membership_fee_start_date do + member.membership_fee_start_date + else + # Calculate from join_date using global settings + include_joining_cycle = get_include_joining_cycle() + + SetMembershipFeeStartDate.calculate_start_date( + member.join_date, + interval, + include_joining_cycle + ) + end + end + + defp determine_end_date(member, today) do + if member.exit_date && Date.compare(member.exit_date, today) == :lt do + # Member has left - use the exit date as boundary + # Note: If exit_date == cycle_start, the cycle IS still generated. + # This means the member is considered a member on the first day of that cycle. + # Example: exit_date = 2025-01-01, yearly interval + # -> The 2025 cycle (starting 2025-01-01) WILL be generated + member.exit_date + else + today + end + end + + defp get_include_joining_cycle do + case Mv.Membership.get_settings() do + {:ok, %{include_joining_cycle: include}} -> include + {:error, _} -> true + end + end + + @doc """ + Generates all cycle start dates from a start date to an end date. + + ## Parameters + + - `start_date` - The first cycle start date + - `end_date` - The date up to which cycles should be generated + - `interval` - The billing interval + + ## Returns + + List of cycle start dates. + + ## Examples + + iex> generate_cycle_starts(~D[2024-01-01], ~D[2024-12-31], :quarterly) + [~D[2024-01-01], ~D[2024-04-01], ~D[2024-07-01], ~D[2024-10-01]] + + """ + @spec generate_cycle_starts(Date.t(), Date.t(), atom()) :: [Date.t()] + def generate_cycle_starts(start_date, end_date, interval) do + # Ensure start_date is aligned to cycle boundary + aligned_start = CalendarCycles.calculate_cycle_start(start_date, interval) + + generate_cycle_starts_acc(aligned_start, end_date, interval, []) + end + + defp generate_cycle_starts_acc(current_start, end_date, interval, acc) do + if Date.compare(current_start, end_date) == :gt do + # Current cycle start is after end date - stop + Enum.reverse(acc) + else + # Include this cycle and continue to next + next_start = CalendarCycles.next_cycle_start(current_start, interval) + generate_cycle_starts_acc(next_start, end_date, interval, [current_start | acc]) + end + end + + defp create_cycles(cycle_starts, member_id, fee_type_id, amount) do + # Always use return_notifications?: true to collect notifications + # Notifications will be returned to the caller, who is responsible for + # sending them (e.g., via after_action hook returning {:ok, result, notifications}) + results = + Enum.map(cycle_starts, fn cycle_start -> + attrs = %{ + cycle_start: cycle_start, + member_id: member_id, + membership_fee_type_id: fee_type_id, + amount: amount, + status: :unpaid + } + + handle_cycle_creation_result( + Ash.create(MembershipFeeCycle, attrs, return_notifications?: true), + cycle_start + ) + end) + + {successes, skips, errors} = + Enum.reduce(results, {[], [], []}, fn + {:ok, cycle, notifications}, {successes, skips, errors} -> + {[{:ok, cycle, notifications} | successes], skips, errors} + + {:skip, cycle_start}, {successes, skips, errors} -> + {successes, [cycle_start | skips], errors} + + {:error, error}, {successes, skips, errors} -> + {successes, skips, [error | errors]} + end) + + all_notifications = + Enum.flat_map(successes, fn {:ok, _cycle, notifications} -> notifications end) + + if Enum.empty?(errors) do + successful_cycles = Enum.map(successes, fn {:ok, cycle, _notifications} -> cycle end) + + if Enum.any?(skips) do + Logger.debug("Skipped #{length(skips)} cycles that already exist for member #{member_id}") + end + + {:ok, successful_cycles, all_notifications} + else + Logger.warning("Some cycles failed to create: #{inspect(errors)}") + # Return partial failure with errors + # Note: When this error occurs, the transaction will be rolled back, + # so no cycles were actually persisted in the database + {:error, {:partial_failure, errors}} + end + end + + defp handle_cycle_creation_result({:ok, cycle, notifications}, _cycle_start) + when is_list(notifications) do + {:ok, cycle, notifications} + end + + defp handle_cycle_creation_result({:ok, cycle}, _cycle_start) do + {:ok, cycle, []} + end + + defp handle_cycle_creation_result( + {:error, + %Ash.Error.Invalid{ + errors: [ + %Ash.Error.Changes.InvalidAttribute{ + private_vars: %{constraint: constraint, constraint_type: :unique} + } + ] + }} = error, + cycle_start + ) do + # Cycle already exists (unique constraint violation) - skip it silently + # This makes the function idempotent and prevents errors on server restart + handle_unique_constraint_violation(constraint, cycle_start, error) + end + + defp handle_cycle_creation_result({:error, reason}, cycle_start) do + {:error, {cycle_start, reason}} + end + + defp handle_unique_constraint_violation( + "membership_fee_cycles_unique_cycle_per_member_index", + cycle_start, + _error + ) do + {:skip, cycle_start} + end + + defp handle_unique_constraint_violation(_constraint, cycle_start, error) do + {:error, {cycle_start, error}} + end +end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index a23381d..ccec5a5 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -95,9 +95,11 @@ defmodule MvWeb.CoreComponents do <.button>Send! <.button phx-click="go" variant="primary">Send! <.button navigate={~p"/"}>Home + <.button disabled={true}>Disabled """ attr :rest, :global, include: ~w(href navigate patch method) attr :variant, :string, values: ~w(primary) + attr :disabled, :boolean, default: false, doc: "Whether the button is disabled" slot :inner_block, required: true def button(%{rest: rest} = assigns) do @@ -105,14 +107,37 @@ defmodule MvWeb.CoreComponents do assigns = assign(assigns, :class, Map.fetch!(variants, assigns[:variant])) if rest[:href] || rest[:navigate] || rest[:patch] do + # For links, we can't use disabled attribute, so we use btn-disabled class + # DaisyUI's btn-disabled provides the same styling as :disabled on buttons + link_class = + if assigns[:disabled], + do: ["btn", assigns.class, "btn-disabled"], + else: ["btn", assigns.class] + + # Prevent interaction when disabled + # Remove navigation attributes to prevent "Open in new tab", "Copy link" etc. + link_attrs = + if assigns[:disabled] do + rest + |> Map.drop([:href, :navigate, :patch]) + |> Map.merge(%{tabindex: "-1", "aria-disabled": "true"}) + else + rest + end + + assigns = + assigns + |> assign(:link_class, link_class) + |> assign(:link_attrs, link_attrs) + ~H""" - <.link class={["btn", @class]} {@rest}> + <.link class={@link_class} {@link_attrs}> {render_slot(@inner_block)} """ else ~H""" - """ @@ -308,7 +333,8 @@ defmodule MvWeb.CoreComponents do attr :error_class, :string, default: nil, doc: "the input error class to use over defaults" attr :rest, :global, - include: ~w(accept autocomplete capture cols disabled form list max maxlength min minlength + include: + ~w(accept autocomplete aria-required capture cols disabled form list max maxlength min minlength multiple pattern placeholder readonly required rows size step) def input(%{field: %Phoenix.HTML.FormField{} = field} = assigns) do @@ -328,6 +354,24 @@ defmodule MvWeb.CoreComponents do Phoenix.HTML.Form.normalize_value("checkbox", assigns[:value]) end) + # For checkboxes, we don't use HTML required attribute (means "must be checked") + # Instead, we use aria-required for screen readers (WCAG 2.1, Success Criterion 3.3.2) + # Extract required from rest and remove it, but keep aria-required if provided + rest = assigns.rest || %{} + is_required = Map.get(rest, :required, false) + aria_required = Map.get(rest, :aria_required, if(is_required, do: "true", else: nil)) + + # Remove required from rest (we don't want HTML required on checkbox) + rest_without_required = Map.delete(rest, :required) + # Ensure aria-required is set if field is required + rest_final = + if aria_required, + do: Map.put(rest_without_required, :aria_required, aria_required), + else: rest_without_required + + assigns = assign(assigns, :rest, rest_final) + assigns = assign(assigns, :is_required, is_required) + ~H"""
diff --git a/lib/mv_web/components/layouts.ex b/lib/mv_web/components/layouts.ex index 487a01f..86090a8 100644 --- a/lib/mv_web/components/layouts.ex +++ b/lib/mv_web/components/layouts.ex @@ -36,12 +36,16 @@ defmodule MvWeb.Layouts do default: nil, doc: "the current [scope](https://hexdocs.pm/phoenix/scopes.html)" + attr :club_name, :string, + default: nil, + doc: "optional club name to pass to navbar" + slot :inner_block, required: true def app(assigns) do ~H""" <%= if @current_user do %> - <.navbar current_user={@current_user} /> + <.navbar current_user={@current_user} club_name={@club_name} /> <% end %>
diff --git a/lib/mv_web/components/layouts/navbar.ex b/lib/mv_web/components/layouts/navbar.ex index 4246c99..adc3444 100644 --- a/lib/mv_web/components/layouts/navbar.ex +++ b/lib/mv_web/components/layouts/navbar.ex @@ -12,15 +12,18 @@ defmodule MvWeb.Layouts.Navbar do required: true, doc: "The current user - navbar is only shown when user is present" - def navbar(assigns) do - club_name = get_club_name() + attr :club_name, :string, + default: nil, + doc: "Optional club name - if not provided, will be loaded from database" + def navbar(assigns) do + club_name = assigns[:club_name] || get_club_name() assigns = assign(assigns, :club_name, club_name) ~H"""
diff --git a/lib/mv_web/live/custom_field_value_live/form.ex b/lib/mv_web/live/custom_field_value_live/form.ex index 4a7b02d..9663927 100644 --- a/lib/mv_web/live/custom_field_value_live/form.ex +++ b/lib/mv_web/live/custom_field_value_live/form.ex @@ -72,7 +72,7 @@ defmodule MvWeb.CustomFieldValueLive.Form do <% end %> <.button phx-disable-with={gettext("Saving...")} variant="primary"> - {gettext("Save Custom field value")} + {gettext("Save Custom Field Value")} <.button navigate={return_path(@return_to, @custom_field_value)}>{gettext("Cancel")} diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index bd57d55..6f7bb54 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -37,7 +37,7 @@ defmodule MvWeb.GlobalSettingsLive do @impl true def render(assigns) do ~H""" - + <.header> {gettext("Settings")} <:subtitle> @@ -88,10 +88,13 @@ defmodule MvWeb.GlobalSettingsLive do @impl true def handle_event("save", %{"setting" => setting_params}, socket) do case AshPhoenix.Form.submit(socket.assigns.form, params: setting_params) do - {:ok, updated_settings} -> + {:ok, _updated_settings} -> + # Reload settings from database to ensure all dependent data is updated + {:ok, fresh_settings} = Membership.get_settings() + socket = socket - |> assign(:settings, updated_settings) + |> assign(:settings, fresh_settings) |> put_flash(:info, gettext("Settings updated successfully")) |> assign_form() diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index 87148ad..53754aa 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -21,6 +21,10 @@ defmodule MvWeb.MemberLive.Form do """ use MvWeb, :live_view + alias Mv.MembershipFees + alias Mv.MembershipFees.MembershipFeeType + alias MvWeb.Helpers.MembershipFeeHelpers + @impl true def render(assigns) do # Sort custom fields by name for display only @@ -144,6 +148,7 @@ defmodule MvWeb.MemberLive.Form do field={value_form[:value]} label={cf.name} type={custom_field_input_type(cf.value_type)} + required={cf.required} /> - <%!-- Payment Data Section (Mockup) --%> + <%!-- Membership Fee Section --%>
- <.form_section title={gettext("Payment Data")}> - - -
-
-