Concept for Groups #354

simon · 2026-01-16T14:56:18+01:00

simon commented

2026-01-16 14:56:18 +01:00

closes #307

Description of the implemented changes

The changes were:

Bugfixing
New Feature
Breaking Change
Refactoring

What has been changed?

Definition of Done

Code Quality

No new technical depths
Linting passed
Documentation is added were needed

Accessibility

New elements are properly defined with html-tags
Colour contrast follows WCAG criteria
Aria labels are added when needed
Everything is accessible by keyboard
Tab-Order is comprehensible
All interactive elements have a visible focus

Testing

Tests for new code are written
All tests pass
axe-core dev tools show no critical or major issues

Additional Notes

closes #307 ## Description of the implemented changes The changes were: - [ ] Bugfixing - [ ] New Feature - [ ] Breaking Change - [ ] Refactoring  ## What has been changed?  ## Definition of Done ### Code Quality - [ ] No new technical depths - [ ] Linting passed - [ ] Documentation is added were needed ### Accessibility - [ ] New elements are properly defined with html-tags - [ ] Colour contrast follows WCAG criteria - [ ] Aria labels are added when needed - [ ] Everything is accessible by keyboard - [ ] Tab-Order is comprehensible - [ ] All interactive elements have a visible focus ### Testing - [ ] Tests for new code are written - [ ] All tests pass - [ ] axe-core dev tools show no critical or major issues ## Additional Notes

simon added 1 commit 2026-01-16 14:56:19 +01:00

docs: add concept for groups af62dbac03

simon added 1 commit 2026-01-16 18:11:00 +01:00

docs: update group concept 1d1f3b16b1

requested reviews from moritz, carla

2026-01-16 18:13:23 +01:00

carla reviewed 2026-01-19 10:43:08 +01:00

docs/groups-architecture.md

					
				@ -0,0 +1132,4 @@

				      assert %{name: ["has already been taken"]} = errors_on(changeset)

				    end

				    test "name uniqueness is case-sensitive" do

carla commented

2026-01-19 10:43:07 +01:00

Do we want this to be case sensitive?

simon commented

2026-01-19 11:08:37 +01:00

I guess not - i'll change it!

simon marked this conversation as resolved

carla approved these changes 2026-01-19 11:11:19 +01:00

carla left a comment

Looks good to me :) Nice!!

docs/groups-architecture.md

					
				@ -0,0 +276,4 @@

				### Search Integration

				**Member Search Enhancement:**

				- Include group names in member search vector

carla commented

2026-01-19 10:48:29 +01:00

Do you think full text in groups is neccessary if we have th efilter or has it consequences for the performance so just a filter is enough as well?

simon commented

2026-01-19 11:21:15 +01:00

I think I expect a fuzzy-fulltext search to match everything that can be found related to a member, including groups. Do you disagree?

carla commented

2026-01-19 11:58:08 +01:00

No I expect the same, I just thought it might affect th eperformance depending on the amount of groups...but let's start with it :)

👍 1

simon marked this conversation as resolved

docs/groups-architecture.md

					
				@ -0,0 +299,4 @@

				- Edit group (inline or modal)

				- Delete group with confirmation modal

				- Show member count per group

carla commented

2026-01-19 10:49:30 +01:00

Add to sidebar settings -> groups?

👍 1

simon marked this conversation as resolved

docs/groups-architecture.md

					
				@ -0,0 +357,4 @@

				### Accessibility (A11y) Considerations

				**Requirements:**

				- All UI elements must be keyboard accessible

carla commented

2026-01-19 11:01:56 +01:00

<3

❤️ 1

simon marked this conversation as resolved

docs/groups-architecture.md Outdated

					
				@ -0,0 +374,4 @@

				</span>

				```

				**Clickable Group Badge (for filtering):**

carla commented

2026-01-19 11:08:30 +01:00

Maybe the dropdown is even enough fo the first start?

👍 1

simon marked this conversation as resolved

docs/groups-architecture.md

					
				@ -0,0 +718,4 @@

				- ❌ Rollen/Positionen in Gruppen

				- ❌ Erweiterte Gruppenattribute (Datum, Status, etc.)

				- ❌ Gruppen-spezifische Berechtigungen

				- ❌ Gruppen in Mitgliedersuche (kann später kommen)

carla commented

2026-01-19 11:10:40 +01:00

I think thats also part of MVP right? :)

simon commented

2026-01-19 11:37:52 +01:00

the search yes, the aspects above I'd leave out for the mvp

think that also matches what we decided in the planning meeting?

the search yes, the aspects above I'd leave out for the mvp think that also matches what we decided in the planning meeting? ![grafik](/attachments/95724d2a-44e4-49d4-8f8c-c68fcb14a679)

grafik.png

18 KiB

carla commented

2026-01-19 11:59:04 +01:00

Yeah, I just meant the last line :)

simon marked this conversation as resolved

simon added 1 commit 2026-01-19 11:53:58 +01:00

docs: update group concept 1c7c56130d

simon added 1 commit 2026-01-19 12:00:07 +01:00

Merge branch 'main' into feature/concept-groups 68baf71119

simon added spent time 2026-01-19 12:01:09 +01:00

30 minutes

simon added 114 commits 2026-01-27 10:55:04 +01:00

feat: adds error capping 257739d273

formatting 6c840e73d1

docs: update implementation plan 3b5225893d

refactor: change length for performance 22593af621

fix: add error message to form 3aa1aee11e

fix: change creation of admin user a161393ea1

fix: linting + tests d8ab9a1015

test: add tdd tests for backend state management of boolean custom filters f052b25213

feat: add custom boolean field state & URL-Parameter 0519433644

test: add tdd tests for custom boolean field filter logic b701b84260

feat: implement filter logic for boolean ustom fields da9ec06e8e

chore: update docs a56895cbba

chore: update docs 926974e2da

docs: Update domain Public API documentation d72bb8869f

refactor: Remove deprecated LiveViews 4154296b54

- Remove CustomFieldValueLive (Index, Form, Show)
- Remove ContributionTypeLive.Index
- Remove ContributionPeriodLive.Show
- Remove corresponding routes from router
- Remove references in CustomFieldValueLive.Index

test: Add tests for UserLive.Show and RoleLive.Show a72896095a

- Add comprehensive tests for UserLive.Show
- Add comprehensive tests for RoleLive.Show
- Cover mount, display, navigation, and error handling

refactor: Replace length/1 with empty list comparison c137ee6221

Replace expensive length/1 calls with direct list comparison
to fix Credo warnings about performance

refactor: Reduce function complexity and nesting depth 4e48ace2d4

- Extract helper functions from process_chunk to reduce nesting
- Extract format_error_message from extract_changeset_error
- Split extract_error_message into smaller functions to reduce complexity
- Fixes Credo refactoring opportunities

chore: update gettext bed2df2d21

i18n: Add missing German translations for member form errors 230fb695d1

- Add translations for validation error messages
- Add translations for save failure messages

test: Remove outdated TODO for auto-assignment feature d296418c8e

Auto-assignment of default membership fee type is already implemented
via SetDefaultMembershipFeeType change. Test assertion is now active.

chore: rm todo list 37c9da54d7

refactor: fix credo issues 7f4c22d072

feat: load boolean custom fields 7171e21a10

feat: add new filter component to members view ca1300f46a

test: add more filter component tests 2dd5ef9926

fix: credo warning be29b46874

fix: resolve pr remarks 0d244a4860

feat: improve filter performance by reducing Ash.read! calls 0016c43c34

fix: failing test 41d680f994

Add System Actor helper for systemic operations 52a482c0e2

Introduce Mv.Helpers.SystemActor module with lazy loading
for operations that must always run regardless of user permissions.
System actor has admin role and auto-creates in test environment.

Create system user in seeds ffe5faa999

Add system@mila.local user with admin role for systemic operations.
This user is used by SystemActor helper for mandatory side effects.

Use system actor for email synchronization 8f06442de9

Update email sync loader and changes to use system actor instead of user actor.
This ensures email sync always works regardless of user permissions.

Use system actor for email uniqueness validation 564e35f65e

Update email validation modules to use system actor for queries.
This ensures data integrity checks always run regardless of user permissions.

Use system actor for cycle generation cf9e6e91fd

Update cycle generator, member hooks, and job to use system actor.
Remove actor parameters as cycle generation is a mandatory side effect.

Add tests for System Actor helper 481e82d541

Test system actor retrieval, caching, fallback behavior,
and auto-creation in test environment.

Document System Actor pattern in code guidelines 8eb05c8a6a

Add section explaining when and how to use system actor for systemic operations.
Include examples and distinction between user mode and system mode.

Add @spec type annotations to SystemActor functions c86aa4ea9c

Add type specifications for all private functions to improve
static analysis with Dialyzer and documentation quality.

Refactor test setup into helper functions 38ae25e0e3

Extract setup code into reusable helper functions to reduce
duplication and improve maintainability.

Replace Mix.env() with Config.sql_sandbox?() in SystemActor 4537b86a09

Use Application config instead of Mix.env() to prevent
runtime crashes in production releases where Mix is not available

Use SystemActor opts for cycle deletion operations fa3c34450d

Pass actor_opts to delete_cycles/1 to ensure proper authorization
when MembershipFeeCycle policies are enforced

Add logging for fail-open email uniqueness validations ce292b431c

Log warnings when query errors occur in email uniqueness checks
to improve visibility of data integrity issues

Make system actor email configurable via SYSTEM_ACTOR_EMAIL 15bf76ab25

Allow system user email to be configured via environment variable
with fallback to default 'system@mila.local'

Add admin authorization check for regenerate cycles button 214b84b9b3

Restrict UI access to cycle regeneration to administrators only
to prevent policy bypass via user interface

Update gettext translations for new UI strings d893c0680a

Move require Logger to module level d9f5579350

Move require Logger statements from function/case level to module level
for better code organization and consistency with Elixir best practices

feat(auth): add User resource authorization policies de187190e4

Implement bypass for READ + HasPermission for UPDATE pattern
Extend HasPermission check to support User resource scope :own

test(auth): add User policies test suite 3ad0db0b2f

31 tests covering all 4 permission sets and bypass scenarios
Update HasPermission tests to expect false for scope :own without record

docs(auth): document User policies and bypass pattern cbc85f8bb8

Add bypass vs HasPermission pattern documentation
Update architecture and implementation plan docs

Harden NoActor check with runtime environment guard 298a13c2e4

Add Mix.env() check to match?/3 for defense in depth.
Document NoActor pattern in CODE_GUIDELINES.md.

Add role loading fallback to HasPermission check 7eb7149e18

Extract ash_resource? helper to reduce nesting depth.
Add ensure_role_loaded fallback for unloaded actor roles.

Clarify User.update :own in permission sets 87e7310cea

Add explicit comments explaining why all permission sets
grant User.update with scope :own for password changes.

Shorten User policy comments to state what only fb5f4990b8

Move why explanations to documentation files.
Keep policy comments concise and focused.

Centralize role preloading in global LiveView on_mount 3852c93ac9

Add ensure_user_role_loaded to global live_view quote block.
Remove redundant on_mount calls from individual LiveViews.

Replace for comprehension with explicit describe blocks 28526deb9c

Fix Credo parsing error by removing for comprehension.
Duplicate tests for own_data, read_only, normal_user sets.

Add PolicyHelpers macro for standard user policies 0d2c8e0905

Encapsulate two-tier policy pattern (bypass + HasPermission).
Promote consistency across resource policy definitions.

Add policy consistency tests d0c1996d6e

Enforce User.update :own across all permission sets.
Verify READ bypass + UPDATE HasPermission pattern.

Update documentation for User credentials strategy 370e5af568

Clarify that User.update :own is handled by HasPermission.
Fix file path references from lib/mv/accounts to lib/accounts.

Replace NoActor runtime Mix.env with compile-time config ab0407abb1

Use Application.compile_env for release-safety.
Config only set in test.exs (defaults to false).

Add centralized Actor.ensure_loaded helper 213521ecf6

Consolidate role loading logic from HasPermission and LiveHelpers.
Use Ash.Resource.Info.resource? for reliable Ash detection.

Remove unused PolicyHelpers macro and PolicyConsistency test 69836978be

Dead code - macro was never used in codebase.
PolicyConsistency test will be replaced with better implementation.

Add authorize?: false to Actor.ensure_loaded 3d753c5460

SECURITY: Skip authorization for role loading to avoid circular dependency.
Actor loads their OWN role, needed for authorization itself.
Documented why this is safe.

Remove skipped get_by_subject test, add explanation e6b22cff16

Test removed - JWT flow tested via AshAuthentication integration.
Direct test would require JWT mocking without value.

Update CODE_GUIDELINES for Application.compile_env pattern 921cc22c09

Replace Mix.env example with config-based approach.
Remove outdated runtime guard documentation.

Fix remaining runtime guard references in CODE_GUIDELINES 726f164b28

Remove mentions of runtime guards - only compile-time config is used.
Clarify that production safety comes from config defaults.

Restrict Actor.ensure_loaded to Mv.Accounts.User only ef4df57a6f

Pattern match on %Mv.Accounts.User{} instead of generic actor.
Clearer intention, prevents accidental authorization bypasses.
Non-User actors are returned as-is (no-op).

Fix authorization bypass in seeds and validations 3051ac9e38

- Add authorize?: false to all bootstrap operations in seeds.exs
- Fix user-linking validation to respect authorize? context flag
- Prevents authorization errors during initial setup when no actor exists yet

Fix OIDC login by using SystemActor in OidcEmailCollision validation fe07a4e955

- Add SystemActor to Ash.read_one() calls in OidcEmailCollision validation
- Prevents authorization failures during OIDC registration when no actor is logged in
- Enables proper email collision detection and account linking flow

Fix OIDC account linking by using SystemActor in LinkOidcAccountLive 21d8d65492

- Add SystemActor to all Ash operations in LinkOidcAccountLive
- Enables user lookup, reload, and oidc_id linking during OIDC flow
- User is not yet logged in during linking, so SystemActor provides authorization

docs: add authorization bootstrap patterns section 8f7f0de822

Document the three authorization bypass mechanisms and when to use each:
- NoActor (test-only bypass)
- system_actor (systemic operations)
- authorize?: false (bootstrap scenarios)

Fix: Ensure members are loaded in handle_params when signature unchanged 36edee6fcc

Remove NoActor bypass from User and Member policies 73dc05c6d4

This removes the NoActor bypass that was masking authorization bugs in tests.
All operations now require an explicit actor for authorization.

Add authorize?: false to SystemActor bootstrap operations 0cedbe52f9

- Role lookup and creation (find_admin_role, create_admin_role)
- System user creation and role assignment
- Role loading during initialization

Add actor parameter to all tests requiring authorization c715a45277

This commit adds actor: system_actor to all Ash operations in tests that
require authorization.

Update documentation: Remove NoActor bypass references b387897adb

Use authorize?: false for integrity checks in validations ba5c982368

Fix tests: Remove duplicate actor keyword arguments 875abc693b

Fix test helpers: Use actor parameter correctly f993c64c42

Fix tests: Add missing actor parameters to Ash operations 0e6b464e0a

Fix tests: Remove redundant system_actor and update test descriptions aedd29c118

Fix test db connections: increase pool size and timeout d807577822

Use admin_user instead of system_actor in LiveView tests 2d7461e36f

Fix rebase conflict: Add actor parameter to helper functions in index_test.exs 31699b7c90

Fix missing actor parameters and restore AshAuthentication bypass tests da0b045771

Remove NoActor module, improve Member validation, update docs ee50f312ee

Remove unused allow_no_actor_bypass config option f426e853de

Add migration to assign 'Mitglied' role to existing users a7e6136648

Add Role helper function and create_role_with_system_flag action e30be4c228

- Add get_mitglied_role/0 helper to avoid code duplication
- Add create_role_with_system_flag action for seeds/migrations
- Allows setting is_system_role flag (required for 'Mitglied' role)

Fix HasPermission check to handle nil member_id gracefully d320cdf14e

Add AssignDefaultRole change for automatic role assignment 4ec90770a4

- Assigns 'Mitglied' role to new users if no role is set

Integrate AssignDefaultRole change into user creation actions aaced70b8e

Update seeds to create all 5 authorization roles cea68638e6

Add test support for default role assignment d77eb4d55e

Add comprehensive tests for default role assignment 885fe613cb

feat: implement attribute-level default for role_id assignment 5164836d32

Replace action-level changes with attribute default function to ensure
all users get the 'Mitglied' role regardless of creation path.

fix: improve migration to create 'Mitglied' role if missing 35fbfe54da

Make migration more robust by creating the 'Mitglied' role if it doesn't
exist, ensuring it works regardless of seed execution order.

fix: bind role_name variable before using in Ash.Query.filter e906f22c4d

Avoid macro pinning issues by binding role_data.name to role_name
before using it in the filter query.

refactor: remove AssignDefaultRole change module f984819508

The attribute-level default solution makes this change module obsolete.
All role assignment is now handled via the role_id attribute's default
function, which is more robust and works for all creation paths.

test: adapt tests for attribute-level default solution 93e966bc6b

chore: increase test timeout and cleanup unused code 047b818ec5

Add NOT NULL constraint to users.role_id and optimize default_role_id 8e519d643d

- Add database-level NOT NULL constraint for users.role_id
- Update SystemActor tests to verify NOT NULL constraint enforcement
- Add process dictionary caching for default_role_id/0 to reduce DB queries

Fix: Don't cache nil in default_role_id to prevent bootstrap issues 793a66639a

- Only cache non-nil role_id values to allow retry after role creation
- Prevents processes from being permanently stuck with nil if first call
  happens before the 'Mitglied' role exists
- Update documentation to explain bootstrap safety mechanism

test: adds live view csv import tests dae9d039c1

fat: adds csv import live view to settings 79d0fa0376

refactor dd68d2efbc

refactor 0acdc82bcc

formatting and refactoring c914e25483

formatting 08534539aa

i18n: add translations f2b363cca5

fix tests and linting 33dc8307c8

Security: Require actor parameter in CSV import 64952d4ff4

Remove fallback to system_actor in process_chunk to prevent
unauthorized access. Actor must now be explicitly provided.

Fix CSV upload file reading 8a5f519099

Handle consume_uploaded_entries returning [content] directly
instead of [{:ok, content}]. Add locale support for translations
in background tasks.

Improve CSV import error messages dc1acb815e

Include email address in duplicate email error messages.
Add German translation for email uniqueness errors.
Ensure locale is set for translations in async tasks.

Fix missing max_errors assign in GlobalSettingsLive

continuous-integration/drone/push Build is failing

Details

2d1ddfa654

Set max_errors as socket assign in mount/3 to make it
available in templates. Fixes KeyError in CSV import UI.

simon closed this pull request

2026-01-27 13:13:36 +01:00

simon referenced this pull request

2026-01-27 13:15:09 +01:00

Add groups concept to docs closes #307 #370

moritz deleted branch feature/concept-groups

2026-01-27 13:22:45 +01:00

continuous-integration/drone/push Build is failing

Details

continuous-integration/drone/promote/production

Required

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Total time spent: 30 minutes

simon

30 minutes

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: local-it/mitgliederverwaltung#354

Rows
Columns