diff --git a/docs/systems/spaces.md b/docs/systems/spaces.md index 6dc22029..be64861b 100644 --- a/docs/systems/spaces.md +++ b/docs/systems/spaces.md @@ -444,8 +444,12 @@ Updates `spaces.ownerId`, broadcasts `space_updated` WS event. Position: `max(existing positions) + 1`. +**Response (201):** the created channel including the creator's computed `myPermissions` and `isPrivate: false` — same shape as the `channel_created` event payload — so the creating client can render it immediately without waiting for the broadcast to round-trip. + **Broadcast:** `channel_created` sent per-user (only to users with VIEW_CHANNEL on the new channel). Each user's event includes their computed `myPermissions`. +**Client reconciliation:** both the create response and the `channel_created` event are applied through the `upsertChannel` store action, which replaces `channels` and `channelPermissions` with fresh references. This is required because the sidebar's `visibleChannels` filter is keyed on `channelPermissions`; mutating that Map in place would set the value without triggering a re-render, leaving a freshly created channel hidden until the space was reopened. + ### Update Channel **Endpoint:** `PATCH /api/channels/:id` diff --git a/packages/server/src/routes/channels.ts b/packages/server/src/routes/channels.ts index 98590b67..01dbaae5 100644 --- a/packages/server/src/routes/channels.ts +++ b/packages/server/src/routes/channels.ts @@ -254,7 +254,15 @@ export async function channelRoutes(app: FastifyInstance): Promise { } } - return reply.code(201).send(channelData); + // Return the channel with the creator's computed permissions (same shape as + // the channel_created WS event) so the client can render it immediately + // without waiting for the broadcast to round-trip. + const creatorPerms = computePermissions(request.userId, id, channelId); + return reply.code(201).send({ + ...channelData, + isPrivate: false, + myPermissions: permissionsToString(creatorPerms), + }); }); // PATCH /api/channels/:id - Update a channel (admin+) diff --git a/packages/web/src/hooks/useWebSocket.ts b/packages/web/src/hooks/useWebSocket.ts index 7cd57db3..9e8826d8 100644 --- a/packages/web/src/hooks/useWebSocket.ts +++ b/packages/web/src/hooks/useWebSocket.ts @@ -1138,20 +1138,7 @@ function handleEvent(origin: string, event: ServerEvent): void { // ─── Channel/space events (all origins) ───────────────────────────────── case 'channel_created': { - const { currentSpaceId: curSpaceId, channels: curChannels, setChannels, channelToSpaceMap, channelPermissions, channelOriginMap, voiceChannelIds } = useSpaceStore.getState(); - if (event.spaceId === curSpaceId) { - if (!curChannels.find(c => c.id === event.channel.id)) { - setChannels([...curChannels, event.channel].sort((a, b) => a.position - b.position)); - } - } - channelToSpaceMap.set(event.channel.id, event.spaceId); - channelOriginMap.set(event.channel.id, origin); - if (event.channel.type === 'voice') { - voiceChannelIds.add(event.channel.id); - } - if (event.channel.myPermissions) { - channelPermissions.set(event.channel.id, event.channel.myPermissions); - } + useSpaceStore.getState().upsertChannel(event.channel, event.spaceId, origin); break; } diff --git a/packages/web/src/stores/spaceStore.ts b/packages/web/src/stores/spaceStore.ts index a24923b6..f093250d 100644 --- a/packages/web/src/stores/spaceStore.ts +++ b/packages/web/src/stores/spaceStore.ts @@ -148,6 +148,7 @@ interface SpaceState { joinByCode: (inviteCode: string, origin?: string) => Promise; generateInvite: (spaceId: string) => Promise; createChannel: (spaceId: string, name: string, type: 'text' | 'voice', topic?: string, categoryId?: string) => Promise; + upsertChannel: (channel: Channel, spaceId: string, origin: string) => void; deleteChannel: (channelId: string) => Promise; createCategory: (spaceId: string, name: string) => Promise; updateCategory: (categoryId: string, data: { name?: string; position?: number }) => Promise; @@ -554,13 +555,46 @@ export const useSpaceStore = create((set, get) => ({ const origin = space?._instanceOrigin ?? ''; const client = getApiForOrigin(origin); const channel = await client.channels.create(spaceId, { name, type, topic, categoryId }); - set((state) => { - if (state.channels.some(c => c.id === channel.id)) return state; - return { channels: [...state.channels, channel].sort((a, b) => a.position - b.position) }; - }); + // Reconcile through upsertChannel so the new channel's permission entry is + // written with a fresh map reference (see upsertChannel). The create + // response carries the creator's myPermissions, so the channel renders + // immediately without waiting for the channel_created WS event. + get().upsertChannel(channel, spaceId, origin); return channel; }, + // Add or replace a channel and its derived lookup state. channels and + // channelPermissions are render inputs for the sidebar's visibleChannels memo, + // so they MUST be replaced with fresh references — mutating the existing + // channelPermissions Map in place sets the value but never triggers a + // re-render, which is why a freshly created channel could stay hidden until + // the space was reloaded. + upsertChannel: (channel: Channel, spaceId: string, origin: string) => { + set((state) => { + // Lookup maps are read imperatively (routing/voice), not render inputs — + // in-place mutation matches their usage everywhere else. + state.channelToSpaceMap.set(channel.id, spaceId); + state.channelOriginMap.set(channel.id, origin); + if (channel.type === 'voice') state.voiceChannelIds.add(channel.id); + + const channelPermissions = new Map(state.channelPermissions); + if (channel.myPermissions) { + channelPermissions.set(channel.id, channel.myPermissions); + } + + // channels only holds the currently-open space's list. + if (state.currentSpaceId !== spaceId) { + return { channelPermissions }; + } + const exists = state.channels.some(c => c.id === channel.id); + const channels = (exists + ? state.channels.map(c => (c.id === channel.id ? channel : c)) + : [...state.channels, channel] + ).sort((a, b) => a.position - b.position); + return { channels, channelPermissions }; + }); + }, + deleteChannel: async (channelId: string) => { const origin = get().channelOriginMap.get(channelId) ?? ''; const channelApi = getApiForOrigin(origin); diff --git a/packages/web/src/stores/spaceStore.upsertChannel.test.ts b/packages/web/src/stores/spaceStore.upsertChannel.test.ts new file mode 100644 index 00000000..334879e0 --- /dev/null +++ b/packages/web/src/stores/spaceStore.upsertChannel.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +vi.mock('../audio/AudioManager', () => ({ + AudioManager: { + getInstance: vi.fn().mockReturnValue({ + setOutputDevice: vi.fn(), + setVolume: vi.fn(), + }), + }, +})); + +vi.mock('./instanceStore', () => ({ + useInstanceStore: Object.assign( + (selector: (s: unknown) => unknown) => selector({ instances: [], _autoConnectDone: true }), + { + getState: () => ({ instances: [], _autoConnectDone: true }), + setState: vi.fn(), + subscribe: vi.fn(), + } + ), +})); + +vi.mock('./authStore', () => ({ + useAuthStore: Object.assign( + (selector: (s: unknown) => unknown) => selector({ user: null, token: null }), + { + getState: () => ({ user: null, token: null }), + setState: vi.fn(), + subscribe: vi.fn(), + } + ), +})); + +import { useSpaceStore } from './spaceStore'; +import type { Channel } from '@backspace/shared'; + +const SPACE = 'space-1'; + +function makeChannel(extras: Partial & Pick): Channel { + return { + spaceId: SPACE, + name: extras.id, + type: 'text', + topic: null, + position: 0, + categoryId: null, + createdAt: 0, + ...extras, + }; +} + +beforeEach(() => { + useSpaceStore.getState().reset(); + useSpaceStore.setState({ currentSpaceId: SPACE }); +}); + +describe('upsertChannel', () => { + it('adds a new channel to the open space with its permission entry', () => { + const ch = makeChannel({ id: 'c1', position: 3, myPermissions: '1' }); + useSpaceStore.getState().upsertChannel(ch, SPACE, ''); + + const s = useSpaceStore.getState(); + expect(s.channels.map(c => c.id)).toEqual(['c1']); + expect(s.channelPermissions.get('c1')).toBe('1'); + expect(s.channelToSpaceMap.get('c1')).toBe(SPACE); + }); + + it('replaces channelPermissions with a FRESH reference (the re-render trigger)', () => { + // This is the regression guard: an in-place Map mutation sets the value but + // never changes identity, so the visibleChannels memo never recomputes. + const before = useSpaceStore.getState().channelPermissions; + useSpaceStore.getState().upsertChannel(makeChannel({ id: 'c1', myPermissions: '1' }), SPACE, ''); + const after = useSpaceStore.getState().channelPermissions; + + expect(after).not.toBe(before); + expect(after.get('c1')).toBe('1'); + }); + + it('reconciles a channel already added optimistically without a permission entry', () => { + // Simulate the race: optimistic create inserted the channel into `channels` + // but no permission was known yet, so it was filtered out of the sidebar. + useSpaceStore.setState({ channels: [makeChannel({ id: 'c1' })] }); + expect(useSpaceStore.getState().channelPermissions.has('c1')).toBe(false); + + // The channel_created event (or the create response) arrives with perms. + useSpaceStore.getState().upsertChannel(makeChannel({ id: 'c1', myPermissions: '1' }), SPACE, ''); + + const s = useSpaceStore.getState(); + expect(s.channels.filter(c => c.id === 'c1')).toHaveLength(1); // no duplicate + expect(s.channelPermissions.get('c1')).toBe('1'); + }); + + it('keeps voice channels out of the channels list updates but tracks them globally', () => { + useSpaceStore.getState().upsertChannel(makeChannel({ id: 'v1', type: 'voice', myPermissions: '1' }), SPACE, ''); + const s = useSpaceStore.getState(); + expect(s.voiceChannelIds.has('v1')).toBe(true); + expect(s.channels.map(c => c.id)).toContain('v1'); + }); + + it('updates lookup maps but not the channels list for a non-open space', () => { + useSpaceStore.getState().upsertChannel( + makeChannel({ id: 'c2', spaceId: 'space-2', myPermissions: '1' }), + 'space-2', + '', + ); + const s = useSpaceStore.getState(); + expect(s.channels.map(c => c.id)).not.toContain('c2'); // different space, list untouched + expect(s.channelToSpaceMap.get('c2')).toBe('space-2'); + expect(s.channelPermissions.get('c2')).toBe('1'); // perms still tracked + }); +});