fix(client): unify role lookups onto the dispatcher-updated store

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 <noreply@anthropic.com>
This commit is contained in:
J3vb
2026-07-20 08:39:01 +02:00
co-authored by Claude Fable 5
parent 5de92b9510
commit 93850689a2
5 changed files with 40 additions and 148 deletions
@@ -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";
@@ -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<RolesState>(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;
}
@@ -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);
});
});
});
@@ -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,
+6 -7
View File
@@ -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`.