From 93850689a2ac695ab6c43fb4fca17a68ff1e11c8 Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Mon, 20 Jul 2026 08:39:01 +0200 Subject: [PATCH] fix(client): unify role lookups onto the dispatcher-updated store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SidebarMemberSection read role name->id from a parallel roles.store that nothing ever wrote to — only channels.store.setRoles is updated by the dispatcher on `ready`. Repoint the reader at channels.store and delete the dead roles.store (its setRoles/getRoleIdByName coverage already lives in channels.store.test.ts). Adds a regression test pinning that the member UI resolves role ids from the store the dispatcher writes. Co-Authored-By: Claude Fable 5 --- .../pages/main-page/SidebarMemberSection.ts | 2 +- Client/tauri-client/src/stores/roles.store.ts | 29 ----- .../tests/unit/roles-store.test.ts | 109 ------------------ .../tests/unit/sidebar-member-section.test.ts | 35 +++++- docs/architecture/ux/channels-members-dms.md | 13 +-- 5 files changed, 40 insertions(+), 148 deletions(-) delete mode 100644 Client/tauri-client/src/stores/roles.store.ts delete mode 100644 Client/tauri-client/tests/unit/roles-store.test.ts diff --git a/Client/tauri-client/src/pages/main-page/SidebarMemberSection.ts b/Client/tauri-client/src/pages/main-page/SidebarMemberSection.ts index fc312031..148e4c9b 100644 --- a/Client/tauri-client/src/pages/main-page/SidebarMemberSection.ts +++ b/Client/tauri-client/src/pages/main-page/SidebarMemberSection.ts @@ -8,7 +8,7 @@ import { createElement, appendChildren } from "@lib/dom"; import type { MountableComponent } from "@lib/safe-render"; import { createMemberList } from "@components/MemberList"; import { authStore } from "@stores/auth.store"; -import { getRoleIdByName } from "@stores/roles.store"; +import { getRoleIdByName } from "@stores/channels.store"; import type { ApiClient } from "@lib/api"; import type { ToastContainer } from "@components/Toast"; diff --git a/Client/tauri-client/src/stores/roles.store.ts b/Client/tauri-client/src/stores/roles.store.ts deleted file mode 100644 index 24c68bf4..00000000 --- a/Client/tauri-client/src/stores/roles.store.ts +++ /dev/null @@ -1,29 +0,0 @@ -/** - * Roles store — holds server-wide role definitions. - * Immutable state updates only. - */ - -import { createStore } from "@lib/store"; -import type { ReadyRole } from "@lib/types"; - -export interface RolesState { - readonly roles: readonly ReadyRole[]; -} - -const INITIAL_STATE: RolesState = { - roles: [], -}; - -export const rolesStore = createStore(INITIAL_STATE); - -/** Bulk set roles from the ready payload. */ -export function setRoles(roles: readonly ReadyRole[]): void { - rolesStore.setState(() => ({ roles })); -} - -/** Look up a role ID by name (case-insensitive). Returns undefined if not found. */ -export function getRoleIdByName(name: string): number | undefined { - const roles = rolesStore.getState().roles; - const match = roles.find((r) => r.name.toLowerCase() === name.toLowerCase()); - return match?.id; -} diff --git a/Client/tauri-client/tests/unit/roles-store.test.ts b/Client/tauri-client/tests/unit/roles-store.test.ts deleted file mode 100644 index 2ec8e59f..00000000 --- a/Client/tauri-client/tests/unit/roles-store.test.ts +++ /dev/null @@ -1,109 +0,0 @@ -import { describe, it, expect, beforeEach } from "vitest"; -import { rolesStore, setRoles, getRoleIdByName } from "../../src/stores/roles.store"; -import type { ReadyRole } from "../../src/lib/types"; - -/** - * Tests for src/stores/roles.store.ts — role definitions store. - * Covers initial state, setRoles bulk update, and getRoleIdByName lookup - * with case-insensitive matching, missing roles, and empty state. - */ - -const SAMPLE_ROLES: readonly ReadyRole[] = [ - { id: 1, name: "admin", color: "#ff0000", permissions: 0xff }, - { id: 2, name: "moderator", color: "#00ff00", permissions: 0x0f }, - { id: 3, name: "member", color: null, permissions: 0x01 }, -]; - -describe("roles.store", () => { - beforeEach(() => { - // Reset to empty state before each test - rolesStore.setState(() => ({ roles: [] })); - }); - - // ── initial state ──────────────────────────────────────── - - describe("initial state", () => { - it("starts with an empty roles array", () => { - expect(rolesStore.getState().roles).toEqual([]); - }); - }); - - // ── setRoles ───────────────────────────────────────────── - - describe("setRoles", () => { - it("bulk sets roles from a ready payload", () => { - setRoles(SAMPLE_ROLES); - expect(rolesStore.getState().roles).toEqual(SAMPLE_ROLES); - }); - - it("replaces existing roles entirely", () => { - setRoles(SAMPLE_ROLES); - - const newRoles: readonly ReadyRole[] = [ - { id: 10, name: "owner", color: "#gold", permissions: 0xffff }, - ]; - setRoles(newRoles); - - expect(rolesStore.getState().roles).toEqual(newRoles); - expect(rolesStore.getState().roles.length).toBe(1); - }); - - it("can set roles to an empty array", () => { - setRoles(SAMPLE_ROLES); - setRoles([]); - expect(rolesStore.getState().roles).toEqual([]); - }); - - it("stores the exact references passed in", () => { - setRoles(SAMPLE_ROLES); - expect(rolesStore.getState().roles).toBe(SAMPLE_ROLES); - }); - }); - - // ── getRoleIdByName ────────────────────────────────────── - - describe("getRoleIdByName", () => { - beforeEach(() => { - setRoles(SAMPLE_ROLES); - }); - - it("returns the role ID for an exact name match", () => { - expect(getRoleIdByName("admin")).toBe(1); - }); - - it("returns the role ID for a case-insensitive match (uppercase)", () => { - expect(getRoleIdByName("ADMIN")).toBe(1); - }); - - it("returns the role ID for a case-insensitive match (mixed case)", () => { - expect(getRoleIdByName("Moderator")).toBe(2); - }); - - it("returns the role ID for a case-insensitive match (lowercase stored, lowercase query)", () => { - expect(getRoleIdByName("member")).toBe(3); - }); - - it("returns undefined for a role name that does not exist", () => { - expect(getRoleIdByName("nonexistent")).toBeUndefined(); - }); - - it("returns undefined when the store is empty", () => { - setRoles([]); - expect(getRoleIdByName("admin")).toBeUndefined(); - }); - - it("returns undefined for an empty string", () => { - expect(getRoleIdByName("")).toBeUndefined(); - }); - - it("returns the first matching role when duplicates exist", () => { - const dupes: readonly ReadyRole[] = [ - { id: 100, name: "DupeRole", color: null, permissions: 0 }, - { id: 200, name: "duperole", color: null, permissions: 0 }, - ]; - setRoles(dupes); - // Should return the first match (id 100) - expect(getRoleIdByName("duperole")).toBe(100); - }); - }); -}); diff --git a/Client/tauri-client/tests/unit/sidebar-member-section.test.ts b/Client/tauri-client/tests/unit/sidebar-member-section.test.ts index adca8987..9636a89b 100644 --- a/Client/tauri-client/tests/unit/sidebar-member-section.test.ts +++ b/Client/tauri-client/tests/unit/sidebar-member-section.test.ts @@ -26,7 +26,7 @@ import { } from "../../src/pages/main-page/SidebarMemberSection"; import { authStore } from "../../src/stores/auth.store"; import { membersStore } from "../../src/stores/members.store"; -import { rolesStore } from "../../src/stores/roles.store"; +import { channelsStore, setRoles } from "../../src/stores/channels.store"; import { createMemberList } from "@components/MemberList"; import type { Member } from "../../src/stores/members.store"; import type { UserStatus } from "../../src/lib/types"; @@ -50,7 +50,11 @@ function resetStores(): void { members: new Map(), typingUsers: new Map(), })); - rolesStore.setState(() => ({ + // Role lookups read the dispatcher-updated channels.store (see the role-source + // unification). Seed via the same setRoles action the dispatcher calls. + channelsStore.setState(() => ({ + channels: new Map(), + activeChannelId: null, roles: [ { id: 1, name: "owner", color: null, permissions: 0 }, { id: 2, name: "admin", color: null, permissions: 0 }, @@ -629,6 +633,33 @@ describe("SidebarMemberSection", () => { section.destroy(); }); + it("changeRole: resolves role id from the dispatcher-updated channels store", async () => { + // Regression guard for the old role-source split: the dispatcher writes + // roles via channels.store.setRoles, so the member UI must read from that + // same store (previously it read a stale, never-updated roles.store). + setRoles([{ id: 42, name: "vip", color: null, permissions: 0 }]); + + const mockApi = { + adminKickMember: vi.fn(), + adminBanMember: vi.fn(), + adminChangeRole: vi.fn().mockResolvedValue(undefined), + }; + const opts = { + api: mockApi as unknown as SidebarMemberSectionOptions["api"], + getToast: vi.fn().mockReturnValue({ show: vi.fn() }), + }; + + const section = createSidebarMemberSection(opts); + container.appendChild(section.element); + + const callbacks = getCapturedCallbacks(); + await callbacks.onChangeRole(7, "Dave", "vip"); + + expect(mockApi.adminChangeRole).toHaveBeenCalledWith(7, 42); + + section.destroy(); + }); + it("uses current user role from auth store", () => { authStore.setState((prev) => ({ ...prev, diff --git a/docs/architecture/ux/channels-members-dms.md b/docs/architecture/ux/channels-members-dms.md index 6236d001..91718b65 100644 --- a/docs/architecture/ux/channels-members-dms.md +++ b/docs/architecture/ux/channels-members-dms.md @@ -95,12 +95,11 @@ affordances covered in [settings-and-admin.md §3](settings-and-admin.md). Actio the user lacks permission for are **not shown** (menu items gated by the actor's role), consistent with the affordance principle. -> **⚠ Current gap — role source split.** Role lookups read from two stores that -> aren't kept in sync: `channels.store` carries `roles`/`getRoleIdByName` (wired -> in the dispatcher) while a parallel `roles.store` exposes the same API, consumed -> by `SidebarMemberSection.ts:11` — only `channels.store.setRoles` is updated by -> `ready` (`dispatcher.ts:10`). Target: one role store, one writer. A stale -> `roles.store` can mis-map a role name→id in the member context menu. +> **✓ Resolved 2026-07-20 — single role store.** Roles live only in +> `channels.store` (`roles`/`getRoleIdByName`), the store the dispatcher writes on +> `ready` (`dispatcher.ts`). `SidebarMemberSection.ts` now reads from it, and the +> parallel, never-updated `roles.store` has been deleted — so the member context +> menu can no longer mis-map a role name→id from stale data. --- @@ -161,5 +160,5 @@ and `IsEitherBlocked` is bidirectional). **Target UX:** `src/components/DmSidebar.ts`, `src/components/DmProfileSidebar.ts`, `src/pages/main-page/SidebarArea.ts`, `SidebarMemberSection.ts`, `SidebarDmSection.ts`, `SidebarDmHelpers.ts`, `src/stores/channels.store.ts`, -`members.store.ts`, `dm.store.ts`, `roles.store.ts`, `src/lib/dispatcher.ts`; +`members.store.ts`, `dm.store.ts`, `src/lib/dispatcher.ts`; server `Server/service/channel.go`, `dm.go`, `block.go`.