From 1d1f3b16b1fe0d1fd561dbdd4180f103223a1476 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 16 Jan 2026 18:10:48 +0100 Subject: [PATCH] docs: update group concept --- docs/groups-architecture.md | 554 +++++++++++++++++++++++++++++++++--- 1 file changed, 513 insertions(+), 41 deletions(-) diff --git a/docs/groups-architecture.md b/docs/groups-architecture.md index 1c04703..88af7de 100644 --- a/docs/groups-architecture.md +++ b/docs/groups-architecture.md @@ -354,6 +354,106 @@ end - Edit group button - Delete group button (with confirmation) +### Accessibility (A11y) Considerations + +**Requirements:** +- All UI elements must be keyboard accessible +- Screen readers must be able to navigate and understand the interface +- ARIA labels and roles must be properly set + +**Group Badges in Member Overview:** + +```heex + + <%= group.name %> + +``` + +**Clickable Group Badge (for filtering):** + +```heex + +``` + +**Group Filter Dropdown:** + +```heex + +``` + +**Screen Reader Announcements:** + +```heex +
+ <%= if @filtered_by_group do %> + Showing <%= @member_count %> members in group <%= @filtered_group.name %> + <% else %> + Showing <%= @member_count %> members + <% end %> +
+``` + +**Delete Confirmation Modal:** + +```heex + +
+

+ Delete Group +

+ +
+

Group "<%= @group.name %>" will be permanently deleted.

+

+ <%= @group.member_count %> members are in this group. + All member-group associations will be permanently deleted. +

+
+ + +
+ +``` + +**Keyboard Navigation:** +- All interactive elements (buttons, links, form inputs) must be focusable via Tab key +- Modal dialogs must trap focus (Tab key cycles within modal) +- Escape key closes modals +- Enter/Space activates buttons when focused + --- ## Integration Points @@ -403,8 +503,7 @@ end - `destroy` - Delete groups (admin only) **Scopes:** -- `:all` - All groups (for admins) -- `:all` - All groups (for read-only users, read permission only) +- `:all` - All groups (for all permission sets that have read access) ### Permission Sets Update @@ -413,10 +512,10 @@ end %{ resources: [ # ... existing resources ... - %{resource: "groups", action: :read, scope: :all, granted: true}, - %{resource: "groups", action: :create, scope: :all, granted: true}, - %{resource: "groups", action: :update, scope: :all, granted: true}, - %{resource: "groups", action: :destroy, scope: :all, granted: true} + %{resource: "Group", action: :read, scope: :all, granted: true}, + %{resource: "Group", action: :create, scope: :all, granted: true}, + %{resource: "Group", action: :update, scope: :all, granted: true}, + %{resource: "Group", action: :destroy, scope: :all, granted: true} ] } ``` @@ -426,11 +525,33 @@ end %{ resources: [ # ... existing resources ... - %{resource: "groups", action: :read, scope: :all, granted: true} + %{resource: "Group", action: :read, scope: :all, granted: true} ] } ``` +**Normal User Permission Set:** +```elixir +%{ + resources: [ + # ... existing resources ... + %{resource: "Group", action: :read, scope: :all, granted: true} + ] +} +``` + +**Own Data Permission Set:** +```elixir +%{ + resources: [ + # ... existing resources ... + %{resource: "Group", action: :read, scope: :all, granted: true} + ] +} +``` + +**Note:** All permission sets use `:all` scope for groups. Groups are considered public information that all users with member read permission can view. Only admins can manage (create/update/destroy) groups. + ### Member-Group Association Permissions **Current (MVP):** @@ -456,13 +577,60 @@ end ### Query Optimization **Member Overview:** -- Load groups with members in single query (using `load`) +- Load groups with members in single query using `Ash.Query.load` - Use `Ash.Query.load(groups: [:id, :name])` to minimize data transfer - Filter groups at database level when filtering by group +**Example:** +```elixir +query = + Mv.Membership.Member + |> Ash.Query.new() + |> Ash.Query.load(groups: [:id, :name]) + |> Ash.Query.filter(expr(groups.id == ^selected_group_id)) + +members = Ash.read!(query, actor: actor) +``` + +**N+1 Query Prevention:** +- Always use `Ash.Query.load` to preload groups relationship +- Never access `member.groups` without preloading (would trigger N+1 queries) + +**Performance Threshold:** +- With proper `load` usage: Works efficiently up to **100 members** (MVP scope) +- For larger datasets (>100 members), consider: + - Pagination (limit number of members loaded) + - Lazy loading of groups (only load when groups column is visible) + - Database-level aggregation for group counts + +**Example of N+1 Problem (DO NOT DO THIS):** +```elixir +# BAD: This causes N+1 queries +members = Ash.read!(Mv.Membership.Member) +Enum.each(members, fn member -> + # Each iteration triggers a separate query! + groups = member.groups # N+1 query! +end) +``` + +**Correct Approach:** +```elixir +# GOOD: Preload in single query +members = + Mv.Membership.Member + |> Ash.Query.load(groups: [:id, :name]) + |> Ash.read!() + +# No additional queries needed +Enum.each(members, fn member -> + groups = member.groups # Already loaded! +end) +``` + **Group Detail:** -- Paginate member list for large groups +- Paginate member list for large groups (>50 members) - Load member count via calculation (not separate query) +- Use `Ash.Query.load` for member details when displaying ### Search Performance @@ -927,48 +1095,352 @@ Jede fachliche Einheit kann als **separates Issue** umgesetzt werden: ### Unit Tests -**Group Resource:** -- Name uniqueness validation -- Name/description length constraints -- Member count calculation +#### Group Resource Tests -**MemberGroup Resource:** -- Unique constraint on (member_id, group_id) -- Cascade delete behavior +**File:** `test/membership/group_test.exs` + +```elixir +defmodule Mv.Membership.GroupTest do + use Mv.DataCase + alias Mv.Membership.Group + + describe "create_group/1" do + test "creates group with valid attributes" do + attrs = %{name: "Vorstand", description: "Board of directors"} + assert {:ok, group} = Group.create(attrs) + assert group.name == "Vorstand" + assert group.description == "Board of directors" + end + + test "returns error when name is missing" do + attrs = %{description: "Some description"} + assert {:error, changeset} = Group.create(attrs) + assert %{name: ["is required"]} = errors_on(changeset) + end + + test "returns error when name exceeds 100 characters" do + long_name = String.duplicate("a", 101) + attrs = %{name: long_name} + assert {:error, changeset} = Group.create(attrs) + assert %{name: ["must be at most 100 character(s)"]} = errors_on(changeset) + end + + test "returns error when name is not unique" do + Group.create!(%{name: "Vorstand"}) + attrs = %{name: "Vorstand"} + assert {:error, changeset} = Group.create(attrs) + assert %{name: ["has already been taken"]} = errors_on(changeset) + end + + test "name uniqueness is case-sensitive" do + Group.create!(%{name: "Vorstand"}) + attrs = %{name: "VORSTAND"} + # For MVP: case-sensitive uniqueness + assert {:ok, _} = Group.create(attrs) + end + + test "allows description to be nil" do + attrs = %{name: "Test Group"} + assert {:ok, group} = Group.create(attrs) + assert is_nil(group.description) + end + + test "trims whitespace from name" do + attrs = %{name: " Vorstand "} + assert {:ok, group} = Group.create(attrs) + assert group.name == "Vorstand" + end + + test "description max length is 500 characters" do + long_desc = String.duplicate("a", 501) + attrs = %{name: "Test", description: long_desc} + assert {:error, changeset} = Group.create(attrs) + assert %{description: ["must be at most 500 character(s)"]} = errors_on(changeset) + end + end + + describe "update_group/2" do + test "updates group name and description" do + group = Group.create!(%{name: "Old Name", description: "Old Desc"}) + attrs = %{name: "New Name", description: "New Desc"} + assert {:ok, updated} = Group.update(group, attrs) + assert updated.name == "New Name" + assert updated.description == "New Desc" + end + + test "prevents duplicate name on update" do + Group.create!(%{name: "Existing"}) + group = Group.create!(%{name: "Other"}) + attrs = %{name: "Existing"} + assert {:error, changeset} = Group.update(group, attrs) + assert %{name: ["has already been taken"]} = errors_on(changeset) + end + end + + describe "delete_group/1" do + test "deletes group and all member associations" do + group = Group.create!(%{name: "Test Group"}) + member = Member.create!(%{email: "test@example.com"}) + MemberGroup.create!(%{member_id: member.id, group_id: group.id}) + + assert :ok = Group.destroy(group) + + # Group should be deleted + assert {:error, _} = Group.get(group.id) + + # MemberGroup association should be deleted (CASCADE) + assert [] = MemberGroup.read!(filter: [group_id: group.id]) + + # Member should still exist + assert {:ok, _} = Member.get(member.id) + end + + test "does not delete members themselves" do + group = Group.create!(%{name: "Test Group"}) + member = Member.create!(%{email: "test@example.com"}) + MemberGroup.create!(%{member_id: member.id, group_id: group.id}) + + Group.destroy!(group) + + # Member should still exist + assert {:ok, _} = Member.get(member.id) + end + end + + describe "member_count calculation" do + test "returns 0 for empty group" do + group = Group.create!(%{name: "Empty Group"}) + assert group.member_count == 0 + end + + test "returns correct count when members added" do + group = Group.create!(%{name: "Test Group"}) + member1 = Member.create!(%{email: "test1@example.com"}) + member2 = Member.create!(%{email: "test2@example.com"}) + + MemberGroup.create!(%{member_id: member1.id, group_id: group.id}) + MemberGroup.create!(%{member_id: member2.id, group_id: group.id}) + + # Reload group to get updated count + group = Group.get!(group.id, load: [:member_count]) + assert group.member_count == 2 + end + + test "updates correctly when members removed" do + group = Group.create!(%{name: "Test Group"}) + member = Member.create!(%{email: "test@example.com"}) + mg = MemberGroup.create!(%{member_id: member.id, group_id: group.id}) + + # Remove member + MemberGroup.destroy!(mg) + + # Reload group + group = Group.get!(group.id, load: [:member_count]) + assert group.member_count == 0 + end + end +end +``` + +#### MemberGroup Resource Tests + +**File:** `test/membership/member_group_test.exs` + +```elixir +defmodule Mv.Membership.MemberGroupTest do + use Mv.DataCase + alias Mv.Membership.{MemberGroup, Member, Group} + + describe "create_member_group/1" do + test "creates association between member and group" do + member = Member.create!(%{email: "test@example.com"}) + group = Group.create!(%{name: "Test Group"}) + + attrs = %{member_id: member.id, group_id: group.id} + assert {:ok, mg} = MemberGroup.create(attrs) + assert mg.member_id == member.id + assert mg.group_id == group.id + end + + test "prevents duplicate associations" do + member = Member.create!(%{email: "test@example.com"}) + group = Group.create!(%{name: "Test Group"}) + MemberGroup.create!(%{member_id: member.id, group_id: group.id}) + + attrs = %{member_id: member.id, group_id: group.id} + assert {:error, changeset} = MemberGroup.create(attrs) + assert %{member_id: ["has already been taken"]} = errors_on(changeset) + end + + test "cascade deletes when member deleted" do + member = Member.create!(%{email: "test@example.com"}) + group = Group.create!(%{name: "Test Group"}) + mg = MemberGroup.create!(%{member_id: member.id, group_id: group.id}) + + Member.destroy!(member) + + # Association should be deleted + assert {:error, _} = MemberGroup.get(mg.id) + end + + test "cascade deletes when group deleted" do + member = Member.create!(%{email: "test@example.com"}) + group = Group.create!(%{name: "Test Group"}) + mg = MemberGroup.create!(%{member_id: member.id, group_id: group.id}) + + Group.destroy!(group) + + # Association should be deleted + assert {:error, _} = MemberGroup.get(mg.id) + end + end +end +``` ### Integration Tests -**Group Management:** -- Create group -- Update group -- Delete group with confirmation -- Add member to group -- Remove member from group +#### Member-Group Relationships -**Member-Group Relationships:** -- Member can belong to multiple groups -- Group can contain multiple members -- Cascade delete when member deleted -- Cascade delete when group deleted +**File:** `test/membership/group_integration_test.exs` + +```elixir +defmodule Mv.Membership.GroupIntegrationTest do + use Mv.DataCase + alias Mv.Membership.{Group, Member, MemberGroup} + + describe "member-group relationships" do + test "member can belong to multiple groups" do + member = Member.create!(%{email: "test@example.com"}) + group1 = Group.create!(%{name: "Group 1"}) + group2 = Group.create!(%{name: "Group 2"}) + + MemberGroup.create!(%{member_id: member.id, group_id: group1.id}) + MemberGroup.create!(%{member_id: member.id, group_id: group2.id}) + + member = Member.get!(member.id, load: [:groups]) + assert length(member.groups) == 2 + assert Enum.any?(member.groups, &(&1.id == group1.id)) + assert Enum.any?(member.groups, &(&1.id == group2.id)) + end + + test "group can contain multiple members" do + group = Group.create!(%{name: "Test Group"}) + member1 = Member.create!(%{email: "test1@example.com"}) + member2 = Member.create!(%{email: "test2@example.com"}) + + MemberGroup.create!(%{member_id: member1.id, group_id: group.id}) + MemberGroup.create!(%{member_id: member2.id, group_id: group.id}) + + group = Group.get!(group.id, load: [:members]) + assert length(group.members) == 2 + end + end +end +``` ### UI Tests -**Groups Management:** -- Groups index page loads -- Create group form works -- Edit group form works -- Delete confirmation modal works -- Name confirmation required for delete +#### Groups Management -**Member Overview:** -- Groups column displays correctly -- Group filter works -- Group sorting works -- URL params persist +**File:** `test/mv_web/live/group_live/index_test.exs` -**Member Detail:** -- Groups section displays -- Links to group pages work +```elixir +defmodule MvWeb.GroupLive.IndexTest do + use MvWeb.ConnCase + alias Mv.Membership.Group + + describe "groups index page" do + test "lists all groups", %{conn: conn} do + group1 = Group.create!(%{name: "Group 1", description: "First group"}) + group2 = Group.create!(%{name: "Group 2", description: "Second group"}) + + {:ok, view, _html} = live(conn, ~p"/groups") + + assert render(view) =~ "Group 1" + assert render(view) =~ "Group 2" + end + + test "creates new group", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/groups") + + view + |> element("button", "New Group") + |> render_click() + + view + |> form("#group-form", group: %{name: "New Group", description: "Description"}) + |> render_submit() + + assert render(view) =~ "New Group" + end + + test "deletes group with confirmation", %{conn: conn} do + group = Group.create!(%{name: "To Delete"}) + + {:ok, view, _html} = live(conn, ~p"/groups") + + # Click delete + view + |> element("button[phx-click='delete']", "Delete") + |> render_click() + + # Enter group name to confirm + view + |> form("#delete-group-modal form", name: "To Delete") + |> render_change() + + # Confirm deletion + view + |> element("#delete-group-modal button", "Delete Group") + |> render_click() + + assert render(view) =~ "Group deleted successfully" + assert {:error, _} = Group.get(group.id) + end + end +end +``` + +#### Member Overview Integration + +**File:** `test/mv_web/live/member_live/index_groups_test.exs` + +```elixir +defmodule MvWeb.MemberLive.IndexGroupsTest do + use MvWeb.ConnCase + alias Mv.Membership.{Member, Group, MemberGroup} + + describe "groups in member overview" do + test "displays group badges", %{conn: conn} do + member = Member.create!(%{email: "test@example.com"}) + group = Group.create!(%{name: "Test Group"}) + MemberGroup.create!(%{member_id: member.id, group_id: group.id}) + + {:ok, view, _html} = live(conn, ~p"/members") + + assert render(view) =~ "Test Group" + end + + test "filters members by group", %{conn: conn} do + member1 = Member.create!(%{email: "test1@example.com"}) + member2 = Member.create!(%{email: "test2@example.com"}) + group = Group.create!(%{name: "Test Group"}) + MemberGroup.create!(%{member_id: member1.id, group_id: group.id}) + + {:ok, view, _html} = live(conn, ~p"/members") + + # Select group filter + view + |> element("#group-filter") + |> render_change(%{"group_filter" => group.id}) + + html = render(view) + assert html =~ "test1@example.com" + refute html =~ "test2@example.com" + end + end +end +``` ---