Merge remote-tracking branch 'origin/main' into feature/ui-for-adding-members-groups
This commit is contained in:
commit
03f27a5938
GPG key ID:
40E7A58C4AA1EDB2
33 changed files with 2765 additions and 501 deletions
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
**Version:** 1.0
|
||||
**Last Updated:** 2026-01-13
|
||||
**Status:** In Progress (Backend Complete, UI Pending)
|
||||
**Status:** In Progress (Backend Complete, UI Complete, Tests Pending)
|
||||
**Related Documents:**
|
||||
- [Feature Roadmap](./feature-roadmap.md) - Overall feature planning
|
||||
|
||||
|
|
@ -15,15 +15,15 @@
|
|||
- ✅ Issue #4: Header Normalization + Per-Header Mapping
|
||||
- ✅ Issue #5: Validation (Required Fields) + Error Formatting
|
||||
- ✅ Issue #6: Persistence via Ash Create + Per-Row Error Capture (with Error-Capping)
|
||||
- ✅ Issue #11: Custom Field Import (Backend)
|
||||
- ✅ Issue #7: Admin Global Settings LiveView UI (Upload + Start Import + Results + Template Links)
|
||||
- ✅ Issue #8: Authorization + Limits
|
||||
- ✅ Issue #11: Custom Field Import (Backend + UI)
|
||||
|
||||
**In Progress / Pending:**
|
||||
- ⏳ Issue #7: Admin Global Settings LiveView UI (Upload + Start Import + Results)
|
||||
- ⏳ Issue #8: Authorization + Limits
|
||||
- ⏳ Issue #9: End-to-End LiveView Tests + Fixtures
|
||||
- ⏳ Issue #10: Documentation Polish
|
||||
|
||||
**Latest Update:** Error-Capping in `process_chunk/4` implemented (2025-01-XX)
|
||||
**Latest Update:** CSV Import UI fully implemented in GlobalSettingsLive with chunk processing, progress tracking, error display, and custom field support (2026-01-13)
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -161,6 +161,13 @@ A **basic CSV member import feature** that allows administrators to upload a CSV
|
|||
- 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.
|
||||
- **Value Validation:** Custom field values are validated according to the custom field type:
|
||||
- **string**: Any text value (trimmed)
|
||||
- **integer**: Must be a valid integer (e.g., `42`, `-10`). Invalid values will cause a row error with the custom field name and reason.
|
||||
- **boolean**: Accepts `true`, `false`, `1`, `0`, `yes`, `no`, `ja`, `nein` (case-insensitive). Invalid values will cause a row error.
|
||||
- **date**: Must be in ISO-8601 format (YYYY-MM-DD, e.g., `2024-01-15`). Invalid values will cause a row error.
|
||||
- **email**: Must be a valid email format (contains `@`, 5-254 characters, valid format). Invalid values will cause a row error.
|
||||
- **Error Messages:** Custom field validation errors are included in the import error list with format: `custom_field: <name> – <reason>` (e.g., `custom_field: Alter – expected integer, got: abc`)
|
||||
|
||||
**Member Field Header Mapping:**
|
||||
|
||||
|
|
@ -496,36 +503,51 @@ Use `Mv.Authorization.PermissionSets` (preferred) instead of hard-coded string c
|
|||
|
||||
**Dependencies:** Issue #6
|
||||
|
||||
**Status:** ✅ **COMPLETED**
|
||||
|
||||
**Goal:** UI section with upload, progress, results, and template links.
|
||||
|
||||
**Tasks:**
|
||||
- [ ] Render import section only for admins
|
||||
- [ ] **Add prominent UI notice about custom fields:**
|
||||
- [x] Render import section only for admins
|
||||
- [x] **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", ...)`:
|
||||
- [x] Configure `allow_upload/3`:
|
||||
- `.csv` only, `max_entries: 1`, `max_file_size: 10MB`, `auto_upload: true` (auto-upload enabled for better UX)
|
||||
- [x] `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)`:
|
||||
- [x] `handle_info({:process_chunk, idx}, socket)`:
|
||||
- Fetch chunk from `import_state`
|
||||
- Call `MemberCSV.process_chunk/3`
|
||||
- Call `MemberCSV.process_chunk/4` with error capping support
|
||||
- Merge counts/errors into progress assigns (cap errors at 50 overall)
|
||||
- Schedule next chunk (or finish and show results)
|
||||
- [ ] Results UI:
|
||||
- Async task processing with SQL sandbox support for tests
|
||||
- [x] 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
|
||||
- Progress indicator during import
|
||||
- Error truncation notice when errors exceed limit
|
||||
|
||||
**Template links:**
|
||||
- Link `/templates/member_import_en.csv` and `/templates/member_import_de.csv` via Phoenix static path helpers.
|
||||
- [x] Link `/templates/member_import_en.csv` and `/templates/member_import_de.csv` via Phoenix static path helpers.
|
||||
|
||||
**Definition of Done:**
|
||||
- [x] Upload area with drag & drop support
|
||||
- [x] Template download links (EN/DE)
|
||||
- [x] Progress tracking during import
|
||||
- [x] Results display with success/error counts
|
||||
- [x] Error list with line numbers and field information
|
||||
- [x] Warning display for unknown custom field columns
|
||||
- [x] Admin-only access control
|
||||
- [x] Async chunk processing with proper error handling
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -533,19 +555,32 @@ Use `Mv.Authorization.PermissionSets` (preferred) instead of hard-coded string c
|
|||
|
||||
**Dependencies:** None (can be parallelized)
|
||||
|
||||
**Status:** ✅ **COMPLETED**
|
||||
|
||||
**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
|
||||
]
|
||||
```
|
||||
- [x] Admin check in start import event handler (via `Authorization.can?/3`)
|
||||
- [x] File size enforced in upload config (`max_file_size: 10MB`)
|
||||
- [x] Row limit enforced in `MemberCSV.prepare/2` (max_rows: 1000, configurable via opts)
|
||||
- [x] Chunk size limit (200 rows per chunk)
|
||||
- [x] Error limit (50 errors per import)
|
||||
- [x] UI-level authorization check (import section only visible to admins)
|
||||
- [x] Event-level authorization check (prevents unauthorized import attempts)
|
||||
|
||||
**Implementation Notes:**
|
||||
- File size limit: 10 MB (10,485,760 bytes) enforced via `allow_upload/3`
|
||||
- Row limit: 1,000 rows (excluding header) enforced in `MemberCSV.prepare/2`
|
||||
- Chunk size: 200 rows per chunk (configurable via opts)
|
||||
- Error limit: 50 errors per import (configurable via `@max_errors`)
|
||||
- Authorization uses `MvWeb.Authorization.can?/3` with `:create` permission on `Mv.Membership.Member`
|
||||
|
||||
**Definition of Done:**
|
||||
- [x] Admin-only access enforced at UI and event level
|
||||
- [x] File size limit enforced
|
||||
- [x] Row count limit enforced
|
||||
- [x] Chunk processing with size limits
|
||||
- [x] Error capping implemented
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -589,7 +624,7 @@ Use `Mv.Authorization.PermissionSets` (preferred) instead of hard-coded string c
|
|||
|
||||
**Priority:** High (Core v1 Feature)
|
||||
|
||||
**Status:** ✅ **COMPLETED** (Backend Implementation)
|
||||
**Status:** ✅ **COMPLETED** (Backend + UI Implementation)
|
||||
|
||||
**Goal:** Support importing custom field values from CSV columns. Custom fields should exist in Mila before import for best results.
|
||||
|
||||
|
|
@ -604,23 +639,26 @@ Use `Mv.Authorization.PermissionSets` (preferred) instead of hard-coded string c
|
|||
- [x] Query existing custom fields during `prepare/2` to map custom field columns
|
||||
- [x] Collect unknown custom field columns and add warning messages (don't fail import)
|
||||
- [x] Map custom field CSV values to `CustomFieldValue` creation in `process_chunk/4`
|
||||
- [x] Handle custom field type validation (string, integer, boolean, date, email)
|
||||
- [x] Handle custom field type validation (string, integer, boolean, date, email) with proper error messages
|
||||
- [x] Create `CustomFieldValue` records linked to members during import
|
||||
- [ ] Update error messages to include custom field validation errors (if needed)
|
||||
- [ ] Add UI help text explaining custom field requirements (pending Issue #7):
|
||||
- [x] Validate custom field values and return structured errors with custom field name and reason
|
||||
- [x] UI help text and link to custom field management (implemented in Issue #7)
|
||||
- [x] Update error messages to include custom field validation errors (format: `custom_field: <name> – expected <type>, got: <value>`)
|
||||
- [x] Add UI help text explaining custom field requirements (completed in Issue #7):
|
||||
- "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 (pending Issue #1)
|
||||
- [x] Update CSV templates documentation to explain custom field columns (documented in Issue #1)
|
||||
- [x] Add tests for custom field import (valid, invalid name, type validation, warning for unknown)
|
||||
|
||||
**Definition of Done:**
|
||||
- [x] Custom field columns are recognized by name (with normalization)
|
||||
- [x] Warning messages shown for unknown custom field columns (import continues)
|
||||
- [x] Custom field values are created and linked to members
|
||||
- [x] Type validation works for all custom field types
|
||||
- [ ] UI clearly explains custom field requirements (pending Issue #7)
|
||||
- [x] Type validation works for all custom field types (string, integer, boolean, date, email)
|
||||
- [x] UI clearly explains custom field requirements (completed in Issue #7)
|
||||
- [x] Tests cover custom field import scenarios (including warning for unknown names)
|
||||
- [x] Error messages include custom field validation errors with proper formatting
|
||||
|
||||
**Implementation Notes:**
|
||||
- Custom field lookup is built in `prepare/2` and passed via `custom_field_lookup` in opts
|
||||
|
|
|
|||
92
docs/page-permission-route-coverage.md
Normal file
92
docs/page-permission-route-coverage.md
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
# Page Permission – Route and Test Coverage
|
||||
|
||||
This document lists all protected routes, which permission set may access them, and how they are covered by tests.
|
||||
|
||||
## Protected Routes (Router scope with CheckPagePermission in :browser)
|
||||
|
||||
| Route | own_data | read_only | normal_user | admin |
|
||||
|-------|----------|-----------|-------------|-------|
|
||||
| `/` | ✗ | ✓ | ✓ | ✓ |
|
||||
| `/members` | ✗ | ✓ | ✓ | ✓ |
|
||||
| `/members/new` | ✗ | ✗ | ✓ | ✓ |
|
||||
| `/members/:id` | ✓ (linked only) | ✓ | ✓ | ✓ |
|
||||
| `/members/:id/edit` | ✓ (linked only) | ✗ | ✓ | ✓ |
|
||||
| `/members/:id/show/edit` | ✓ (linked only) | ✗ | ✓ | ✓ |
|
||||
| `/users` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/users/new` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/users/:id` | ✓ (own only) | ✓ (own only) | ✓ (own only) | ✓ |
|
||||
| `/users/:id/edit` | ✓ (own only) | ✓ (own only) | ✓ (own only) | ✓ |
|
||||
| `/users/:id/show/edit` | ✓ (own only) | ✓ (own only) | ✓ (own only) | ✓ |
|
||||
| `/settings` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/membership_fee_settings` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/membership_fee_types` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/membership_fee_types/new` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/membership_fee_types/:id/edit` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/groups` | ✗ | ✓ | ✓ | ✓ |
|
||||
| `/groups/new` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/groups/:slug` | ✗ | ✓ | ✓ | ✓ |
|
||||
| `/groups/:slug/edit` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/admin/roles` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/admin/roles/new` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/admin/roles/:id` | ✗ | ✗ | ✗ | ✓ |
|
||||
| `/admin/roles/:id/edit` | ✗ | ✗ | ✗ | ✓ |
|
||||
|
||||
**Note:** Permission sets define `/custom_field_values` and related paths, but there are no such routes in the router; those entries are for future use.
|
||||
|
||||
## Public Paths (no permission check)
|
||||
|
||||
- `/auth*`, `/register`, `/reset`, `/sign-in`, `/sign-out`, `/confirm*`, `/password-reset*`, `/set_locale`
|
||||
|
||||
## Test Coverage
|
||||
|
||||
**File:** `test/mv_web/plugs/check_page_permission_test.exs`
|
||||
|
||||
### Unit tests (plug called directly with mock conn)
|
||||
|
||||
- Static: own_data denied `/members`; read_only allowed `/members`; flash on denial.
|
||||
- Dynamic: read_only allowed `/members/123`; normal_user allowed `/members/456/edit`; read_only denied `/members/123/edit`.
|
||||
- read_only / normal_user: denied `/admin/roles`; read_only denied `/members/new`.
|
||||
- Wildcard: admin allowed `/admin/roles`, `/members/999/edit`.
|
||||
- Unauthenticated: nil user denied, redirect `/sign-in`.
|
||||
- Public: unauthenticated allowed `/auth/sign-in`, `/register`.
|
||||
- Error: no role, invalid permission_set_name → denied.
|
||||
|
||||
### Integration tests (full router, Mitglied = own_data)
|
||||
|
||||
**Denied (Mitglied gets 302 → `/users/:id`):**
|
||||
|
||||
- `/members`, `/members/new`, `/users`, `/users/new`, `/settings`, `/membership_fee_settings`, `/membership_fee_types`, `/membership_fee_types/new`, `/groups`, `/groups/new`, `/admin/roles`, `/admin/roles/new`
|
||||
- `/members/:id/edit`, `/members/:id/show/edit`, `/users/:id` (other user), `/users/:id/edit` (other), `/users/:id/show/edit` (other), `/membership_fee_types/:id/edit`, `/groups/:slug`, `/admin/roles/:id`, `/admin/roles/:id/edit`
|
||||
|
||||
**Allowed (Mitglied gets 200):**
|
||||
|
||||
- `/users/:id` (own profile), `/users/:id/edit`, `/users/:id/show/edit`
|
||||
- `/members/:id`, `/members/:id/edit`, `/members/:id/show/edit` for linked member (plug unit tests; full-router tests for linked member skipped: session/LiveView constraints)
|
||||
|
||||
**Root:** `GET /` redirects Mitglied to profile (root not allowed for own_data).
|
||||
|
||||
All protected routes above are either covered by integration “denied” tests for Mitglied or by unit tests for the relevant permission set.
|
||||
|
||||
### Integration tests (full router, read_only = Vorstand/Buchhaltung)
|
||||
|
||||
**Allowed (200):** `/`, `/members`, `/members/:id`, `/users/:id` (own profile), `/users/:id/edit`, `/users/:id/show/edit`, `/groups`, `/groups/:slug`.
|
||||
|
||||
**Denied (302 → `/users/:id`):** `/members/new`, `/members/:id/edit`, `/members/:id/show/edit`, `/users`, `/users/new`, `/users/:id` (other user), `/settings`, `/membership_fee_settings`, `/membership_fee_types`, `/groups/new`, `/groups/:slug/edit`, `/admin/roles`, `/admin/roles/:id`.
|
||||
|
||||
### Integration tests (full router, normal_user = Kassenwart)
|
||||
|
||||
**Allowed (200):** `/`, `/members`, `/members/new`, `/members/:id`, `/members/:id/edit`, `/members/:id/show/edit`, `/users/:id` (own profile), `/users/:id/edit`, `/users/:id/show/edit`, `/groups`, `/groups/:slug`.
|
||||
|
||||
**Denied (302 → `/users/:id`):** `/users`, `/users/new`, `/users/:id` (other user), `/settings`, `/membership_fee_settings`, `/membership_fee_types`, `/groups/new`, `/groups/:slug/edit`, `/admin/roles`, `/admin/roles/:id`.
|
||||
|
||||
### Integration tests (full router, admin)
|
||||
|
||||
**Allowed (200):** All protected routes (sample covered: `/`, `/members`, `/users`, `/settings`, `/membership_fee_settings`, `/admin/roles`, `/members/:id`, `/admin/roles/:id`, `/groups/:slug`).
|
||||
|
||||
## Plug behaviour: reserved segments
|
||||
|
||||
The plug treats `"new"` as a reserved path segment so that patterns like `/members/:id` and `/groups/:slug` do not match `/members/new` or `/groups/new`. Thus `/groups/new` is only allowed when the permission set explicitly lists `/groups/new` (currently only admin).
|
||||
|
||||
## Role and member_id loading
|
||||
|
||||
The plug may reload the user's role (and optionally `member_id`) before checking page permission. Session/`load_from_session` can leave the role unloaded; the plug uses `Mv.Authorization.Actor.ensure_loaded/1` (and, when needed, loads `member_id`) so that permission checks always have the required data. No change to session loading is required; this is documented for clarity.
|
||||
|
|
@ -2002,6 +2002,8 @@ Users and Members are separate entities that can be linked. Special rules:
|
|||
- A user cannot link themselves to an existing member
|
||||
- A user CAN create a new member and be directly linked to it (self-service)
|
||||
|
||||
**Enforcement:** The User resource restricts the `update_user` action (which accepts the `member` argument for link/unlink) to admins only via `Mv.Authorization.Checks.ActorIsAdmin`. The UserLive.Form shows the Member-Linking UI and runs member link/unlink on save only when the current user is admin; non-admins use the `:update` action (email only) for profile edit.
|
||||
|
||||
### Approach: Separate Ash Actions
|
||||
|
||||
We use **different Ash actions** to enforce different policies:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue