diff --git a/apps/api/src/__tests__/messaging-channels-permissions.test.ts b/apps/api/src/__tests__/messaging-channels-permissions.test.ts new file mode 100644 index 00000000..5a2d8417 --- /dev/null +++ b/apps/api/src/__tests__/messaging-channels-permissions.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect } from 'vitest'; +import type { Kysely } from 'kysely'; +import type { Database } from '@vencore/db'; +import type { RequestHandler } from 'express'; +import { createChannelsRouter } from '../routes/messaging/channels'; +import { MESSAGING_MODULE } from '@vencore/modules'; + +/** + * Tags each gate handler with the permission it was built from, so the assertions + * can read the gate off the router without standing up Express + a database. + */ +function taggingRequirePermission(permission: string): RequestHandler { + const handler = (() => undefined) as unknown as RequestHandler; + (handler as unknown as { __permission: string }).__permission = permission; + return handler; +} + +function permissionsForRoute(router: unknown, method: string, path: string): string[] { + const stack = (router as { stack: unknown[] }).stack; + const perms: string[] = []; + for (const layer of stack as Array<{ + route?: { path: string; methods: Record; stack: Array<{ handle: { __permission?: string } }> }; + }>) { + if (!layer.route) continue; + if (layer.route.path !== path) continue; + if (!layer.route.methods[method]) continue; + for (const h of layer.route.stack) { + const p = h.handle.__permission; + if (p) perms.push(p); + } + } + return perms; +} + +describe('messaging channels route permissions', () => { + const db = {} as Kysely; + + it('gates channel creation on create_channel, not the admin-only manage', () => { + const router = createChannelsRouter(db, taggingRequirePermission); + + const perms = permissionsForRoute(router, 'post', '/'); + + expect(perms).toContain('messaging:create_channel'); + expect(perms).not.toContain('messaging:manage'); + }); + + it('still gates rename and archive on manage', () => { + const router = createChannelsRouter(db, taggingRequirePermission); + + expect(permissionsForRoute(router, 'patch', '/:id')).toContain('messaging:manage'); + expect(permissionsForRoute(router, 'delete', '/:id')).toContain('messaging:manage'); + }); + + it('keeps listing and reading channels on view', () => { + const router = createChannelsRouter(db, taggingRequirePermission); + + expect(permissionsForRoute(router, 'get', '/')).toContain('messaging:view'); + expect(permissionsForRoute(router, 'get', '/:id')).toContain('messaging:view'); + }); +}); + +describe('messaging module permission definitions', () => { + const byKey = new Map(MESSAGING_MODULE.permissions.map(p => [p.key, p])); + + it('declares messaging:create_channel', () => { + expect(byKey.has('messaging:create_channel')).toBe(true); + }); + + it('grants channel creation to members, so they are not locked out', () => { + expect(byKey.get('messaging:create_channel')?.defaultRoles).toContain('member'); + expect(byKey.get('messaging:create_channel')?.defaultRoles).toContain('admin'); + }); + + it('keeps messaging:manage admin-only', () => { + expect(byKey.get('messaging:manage')?.defaultRoles).toEqual(['admin']); + }); +}); diff --git a/apps/api/src/routes/messaging/channels.ts b/apps/api/src/routes/messaging/channels.ts index aa0c402e..14ba6473 100644 --- a/apps/api/src/routes/messaging/channels.ts +++ b/apps/api/src/routes/messaging/channels.ts @@ -141,18 +141,40 @@ export function createChannelsRouter( } } + // DM rows are all stored as name='dm'; the display name is built from the + // participants. Without members here the sidebar renders every DM + // identically, so attach them for dm/group_dm rows only. + const dmIds = rows.filter(r => r.type === 'dm' || r.type === 'group_dm').map(r => r.id); + const dmMembers = dmIds.length + ? await db + .selectFrom('channel_members') + .innerJoin('users', 'users.id', 'channel_members.user_id') + .where('channel_members.channel_id', 'in', dmIds) + .select(['channel_members.channel_id', 'channel_members.user_id', 'users.name', 'users.email']) + .execute() + : []; + + const membersByChannel = new Map(); + for (const m of dmMembers) { + const list = membersByChannel.get(m.channel_id) ?? []; + list.push({ user_id: m.user_id, name: m.name, email: m.email }); + membersByChannel.set(m.channel_id, list); + } + const channels = rows.map(r => ({ ...r, unread_count: unreadCounts[r.id] ?? 0, last_message: latestMap.get(r.id) ?? null, + ...(membersByChannel.has(r.id) ? { members: membersByChannel.get(r.id) } : {}), })); res.json({ data: channels, error: null }); } catch (err) { next(err); } }); - // Create channel - router.post('/', requirePermission('messaging:manage'), async (req, res, next) => { + // Create channel. Gated on create_channel, not manage — 'manage' is admin-only + // and would stop ordinary members from ever creating a channel. + router.post('/', requirePermission('messaging:create_channel'), async (req, res, next) => { try { const { workspace, user } = req as unknown as AuthenticatedRequest; const body = createChannelSchema.parse(req.body); diff --git a/apps/api/src/routes/tasks-unified.ts b/apps/api/src/routes/tasks-unified.ts index 3f0d1382..d91fa50b 100644 --- a/apps/api/src/routes/tasks-unified.ts +++ b/apps/api/src/routes/tasks-unified.ts @@ -85,7 +85,7 @@ export function createUnifiedTasksRouter(db: Kysely, requirePermission return } const { status, source, priority, show_all, q, owner_id } = parsed.data - const showAll = show_all && user.role === 'admin' + const showAll = show_all && isAdmin // ── 1. CRM tasks ──────────────────────────────────────────────────────── let crmQ = db diff --git a/apps/web/modules/messaging/components/ChannelSidebar.tsx b/apps/web/modules/messaging/components/ChannelSidebar.tsx index a2387467..635cfbd7 100644 --- a/apps/web/modules/messaging/components/ChannelSidebar.tsx +++ b/apps/web/modules/messaging/components/ChannelSidebar.tsx @@ -9,13 +9,20 @@ import { ContextMenu, useContextMenu, type ContextMenuItem } from '@/modules/sha import { useConfirm } from '@/modules/shared/components/ui/ConfirmDialog'; import { useApiToken } from '@/modules/shared/lib/useApiToken'; import { listChannels, createChannel, updateChannel, archiveChannel, markChannelRead } from '../lib/messaging'; +import { channelDisplayName, type ChannelMemberSummary } from '../lib/dm-name'; +import { useAuth } from '@/modules/shared/lib/AuthContext'; import { NewDMModal } from './NewDMModal'; import type { Channel, Message } from '@vencore/types'; -type ChannelWithMeta = Channel & { unread_count: number; last_message: Message | null }; +type ChannelWithMeta = Channel & { + unread_count: number; + last_message: Message | null; + members?: ChannelMemberSummary[]; +}; export function ChannelSidebar() { const getToken = useApiToken(); + const { user } = useAuth(); const router = useRouter(); const params = useParams(); const activeId = params?.['channelId'] as string | undefined; @@ -163,6 +170,7 @@ export function ChannelSidebar() { active={activeId === ch.id} onClick={() => router.push(`/messaging/${ch.id}`)} onMarkRead={ch.last_message ? () => markRead.mutate({ id: ch.id, messageId: ch.last_message!.id }) : undefined} + displayName={channelDisplayName(ch, user?.id ?? '')} isDm /> ))} @@ -232,12 +240,14 @@ function SectionLabel({ label }: { label: string }) { } function ChannelRow({ - channel, active, onClick, isDm, onOpenDetails, onRename, onArchive, onMarkRead, + channel, active, onClick, isDm, displayName, onOpenDetails, onRename, onArchive, onMarkRead, }: { channel: ChannelWithMeta; active: boolean; onClick: () => void; isDm?: boolean; + /** DMs are stored as name='dm'; the row shows the other participant instead. */ + displayName?: string; onOpenDetails?: () => void; onRename?: (name: string) => void; onArchive?: () => void; @@ -323,7 +333,7 @@ function ChannelRow({ {isDm ? '●' : channel.is_private ? : '#'} - {channel.name} + {displayName ?? channel.name} {hasUnread && !active && ( : '#'} - {channelData?.name ?? '…'} + {channelData ? channelDisplayName(channelData, user?.id ?? '') : '…'} {channelData?.topic && ( <> @@ -131,7 +132,7 @@ export function ChannelView({ channelId }: Props) { diff --git a/apps/web/modules/messaging/lib/dm-name.test.ts b/apps/web/modules/messaging/lib/dm-name.test.ts new file mode 100644 index 00000000..457baf0b --- /dev/null +++ b/apps/web/modules/messaging/lib/dm-name.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect } from 'vitest'; +import { channelDisplayName } from './dm-name'; + +const ME = 'user-me'; + +describe('channelDisplayName', () => { + it('returns the stored name for a regular channel', () => { + expect(channelDisplayName({ name: 'general', type: 'channel' }, ME)).toBe('general'); + }); + + it('ignores members on a regular channel', () => { + const channel = { + name: 'engineering', + type: 'channel', + members: [{ user_id: 'user-other', name: 'Admin' }], + }; + expect(channelDisplayName(channel, ME)).toBe('engineering'); + }); + + it('names a DM after the other participant, not "dm"', () => { + const channel = { + name: 'dm', + type: 'dm', + members: [ + { user_id: ME, name: 'Claude Test' }, + { user_id: 'user-other', name: 'Admin' }, + ], + }; + expect(channelDisplayName(channel, ME)).toBe('Admin'); + }); + + it('joins all other participants for a group DM', () => { + const channel = { + name: 'dm', + type: 'group_dm', + members: [ + { user_id: ME, name: 'Claude Test' }, + { user_id: 'u2', name: 'Admin' }, + { user_id: 'u3', name: 'Dana' }, + ], + }; + expect(channelDisplayName(channel, ME)).toBe('Admin, Dana'); + }); + + it('falls back to a readable label when members are not loaded', () => { + expect(channelDisplayName({ name: 'dm', type: 'dm' }, ME)).toBe('Direct message'); + }); + + it('falls back when the only member is yourself', () => { + const channel = { + name: 'dm', + type: 'dm', + members: [{ user_id: ME, name: 'Claude Test' }], + }; + expect(channelDisplayName(channel, ME)).toBe('Direct message'); + }); + + it('returns empty string for a missing channel', () => { + expect(channelDisplayName(null, ME)).toBe(''); + }); +}); diff --git a/apps/web/modules/messaging/lib/dm-name.ts b/apps/web/modules/messaging/lib/dm-name.ts new file mode 100644 index 00000000..71459a6b --- /dev/null +++ b/apps/web/modules/messaging/lib/dm-name.ts @@ -0,0 +1,33 @@ +export interface ChannelMemberSummary { + user_id: string; + name: string; + email?: string; +} + +export interface NameableChannel { + name: string; + type?: string | null; + members?: ChannelMemberSummary[] | undefined; +} + +/** + * DM channels are all stored with name='dm' — the display name is derived from + * the other participants. Regular channels keep their stored name. + */ +export function channelDisplayName( + channel: NameableChannel | null | undefined, + currentUserId: string, +): string { + if (!channel) return ''; + + const isDm = channel.type === 'dm' || channel.type === 'group_dm'; + if (!isDm) return channel.name; + + const others = (channel.members ?? []).filter(m => m.user_id !== currentUserId); + if (others.length === 0) { + // Members not loaded yet, or a DM with only yourself left in it. + return channel.name === 'dm' ? 'Direct message' : channel.name; + } + + return others.map(m => m.name).join(', '); +} diff --git a/docker-compose.yml b/docker-compose.yml index 88e43d43..de7b5e58 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -55,6 +55,10 @@ services: api: image: ${API_IMAGE:-ghcr.io/vencorehq/vencore-api:latest} + # Published so a locally-run `pnpm dev` web server can reach the API through + # the Next.js rewrite (and so WS upgrades can bypass Next entirely). + ports: + - "127.0.0.1:3001:3001" env_file: .env environment: DATABASE_URL: postgresql://${DB_USER:-vencore}:${DB_PASSWORD:-vencore}@db:5432/${DB_NAME:-vencore} diff --git a/packages/db/migrations/20260803_001_messaging_create_channel_permission.ts b/packages/db/migrations/20260803_001_messaging_create_channel_permission.ts new file mode 100644 index 00000000..8785be66 --- /dev/null +++ b/packages/db/migrations/20260803_001_messaging_create_channel_permission.ts @@ -0,0 +1,25 @@ +import { type Kysely, sql } from 'kysely'; + +// Adding a permission to a ModuleDefinition only reaches NEW workspaces: +// seedWorkspaceRoles grants module defaults when a workspace's Member role is +// first created and never runs again. Existing workspaces therefore need an +// explicit backfill, or members stay locked out of channel creation. +// +// Backfill rule: any role that can already send messages can create channels. +// Roles with grants_all are unaffected — they short-circuit to superuser and +// never consult role_permissions. +export async function up(db: Kysely): Promise { + await sql` + insert into role_permissions (workspace_id, role_id, permission) + select workspace_id, role_id, 'messaging:create_channel' + from role_permissions + where permission = 'messaging:send' + on conflict (role_id, permission) do nothing + `.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql` + delete from role_permissions where permission = 'messaging:create_channel' + `.execute(db); +} diff --git a/packages/modules/src/messaging/index.ts b/packages/modules/src/messaging/index.ts index 05e3336a..536c3f89 100644 --- a/packages/modules/src/messaging/index.ts +++ b/packages/modules/src/messaging/index.ts @@ -7,9 +7,13 @@ export const MESSAGING_MODULE: ModuleDefinition = { icon: 'MessageSquare', defaultEnabled: true, permissions: [ - { key: 'messaging:view', label: 'View channels and messages', defaultRoles: ['admin', 'member'] }, - { key: 'messaging:send', label: 'Send messages and upload files', defaultRoles: ['admin', 'member'] }, - { key: 'messaging:manage', label: 'Manage channels and delete messages', defaultRoles: ['admin'] }, + { key: 'messaging:view', label: 'View channels and messages', defaultRoles: ['admin', 'member'] }, + { key: 'messaging:send', label: 'Send messages and upload files', defaultRoles: ['admin', 'member'] }, + // Creating a channel is a normal member action; renaming/archiving someone + // else's channel is not. Keeping them separate means members are not locked + // out of creating channels by the admin-only 'manage' gate. + { key: 'messaging:create_channel', label: 'Create channels', defaultRoles: ['admin', 'member'] }, + { key: 'messaging:manage', label: 'Manage channels and delete messages', defaultRoles: ['admin'] }, ], nav: [{ label: 'Messaging', path: '/messaging', icon: 'MessageSquare' }], apiPrefixes: ['/messaging'],