Skip to content

Commit 938d1bc

Browse files
waleedlatif1waleedlatif1
authored
fix(sidebar): align organization and workspace navigation (#7777)
* fix(sidebar): align organization and workspace navigation * fix(settings): classify recently deleted in workspace move impact * fix(tests): isolate organization layout event subscriptions
1 parent 4d1c9d0 commit 938d1bc

67 files changed

Lines changed: 2912 additions & 612 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/sim/app/api/copilot/chat/stop/route.test.ts‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ vi.mock('@/lib/copilot/chat/messages-store', () => ({
2121
}))
2222

2323
vi.mock('@/lib/copilot/chat-status', () => ({
24-
chatPubSub: {
25-
publishStatusChanged: mockPublishStatusChanged,
26-
},
24+
publishChatStatusChanged: mockPublishStatusChanged,
2725
}))
2826

2927
import { POST } from '@/app/api/copilot/chat/stop/route'
@@ -59,7 +57,7 @@ describe('copilot chat stop route', () => {
5957
user: { id: 'user-1' },
6058
session: { id: 'session-1' },
6159
})
62-
mockGetAccessibleChat.mockResolvedValue({ id: 'chat-1' })
60+
mockGetAccessibleChat.mockResolvedValue({ id: 'chat-1', workspaceId: 'ws-1', userId: 'user-1' })
6361
})
6462

6563
it('does not persist stopped content after organization access is removed', async () => {
@@ -120,12 +118,14 @@ describe('copilot chat stop route', () => {
120118
contentBlocks: [{ type: 'complete', status: 'cancelled' }],
121119
})
122120

123-
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
124-
workspaceId: 'ws-1',
125-
chatId: 'chat-1',
126-
type: 'completed',
127-
streamId: 'stream-1',
128-
})
121+
expect(mockPublishStatusChanged).toHaveBeenCalledWith(
122+
expect.objectContaining({ workspaceId: 'ws-1' }),
123+
{
124+
chatId: 'chat-1',
125+
type: 'completed',
126+
streamId: 'stream-1',
127+
}
128+
)
129129
})
130130

131131
it('appends a stopped assistant message if the stream marker was already cleared', async () => {
@@ -145,12 +145,14 @@ describe('copilot chat stop route', () => {
145145
const [, appended] = mockAppendCopilotChatMessages.mock.calls[0]
146146
expect(appended[0]).toMatchObject({ role: 'assistant', content: 'partial' })
147147

148-
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
149-
workspaceId: 'ws-1',
150-
chatId: 'chat-1',
151-
type: 'completed',
152-
streamId: 'stream-1',
153-
})
148+
expect(mockPublishStatusChanged).toHaveBeenCalledWith(
149+
expect.objectContaining({ workspaceId: 'ws-1' }),
150+
{
151+
chatId: 'chat-1',
152+
type: 'completed',
153+
streamId: 'stream-1',
154+
}
155+
)
154156
})
155157

156158
it('republishes completed status when the assistant was already persisted', async () => {
@@ -167,11 +169,13 @@ describe('copilot chat stop route', () => {
167169
expect(await response.json()).toEqual({ success: true })
168170
expect(mockAppendCopilotChatMessages).not.toHaveBeenCalled()
169171
expect(dbChainMockFns.set).not.toHaveBeenCalled()
170-
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
171-
workspaceId: 'ws-1',
172-
chatId: 'chat-1',
173-
type: 'completed',
174-
streamId: 'stream-1',
175-
})
172+
expect(mockPublishStatusChanged).toHaveBeenCalledWith(
173+
expect.objectContaining({ workspaceId: 'ws-1' }),
174+
{
175+
chatId: 'chat-1',
176+
type: 'completed',
177+
streamId: 'stream-1',
178+
}
179+
)
176180
})
177181
})

‎apps/sim/app/api/copilot/chat/stop/route.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
withStoppedContentBlock,
1212
} from '@/lib/copilot/chat/persisted-message'
1313
import { finalizeAssistantTurn } from '@/lib/copilot/chat/terminal-state'
14-
import { chatPubSub } from '@/lib/copilot/chat-status'
14+
import { publishChatStatusChanged } from '@/lib/copilot/chat-status'
1515
import {
1616
CopilotChatFinalizeOutcome,
1717
CopilotStopOutcome,
@@ -87,9 +87,8 @@ export const POST = withRouteHandler((req: NextRequest) =>
8787
const shouldPublishCompleted =
8888
result.updated || result.outcome === CopilotChatFinalizeOutcome.AssistantAlreadyPersisted
8989

90-
if (shouldPublishCompleted && result.workspaceId) {
91-
chatPubSub?.publishStatusChanged({
92-
workspaceId: result.workspaceId,
90+
if (shouldPublishCompleted) {
91+
publishChatStatusChanged(chat, {
9392
chatId,
9493
type: 'completed',
9594
streamId,

‎apps/sim/app/api/mothership/chats/[chatId]/fork/route.test.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ vi.mock('@/lib/copilot/chat/messages-store', () => ({
6565
}))
6666

6767
vi.mock('@/lib/copilot/chat-status', () => ({
68-
chatPubSub: { publishStatusChanged: mockPublishStatusChanged },
68+
publishChatStatusChanged: mockPublishStatusChanged,
6969
}))
7070

7171
vi.mock('@/lib/copilot/request/go/fetch', () => ({
@@ -291,11 +291,13 @@ describe('POST /api/mothership/chats/[chatId]/fork', () => {
291291
userId: 'user-1',
292292
})
293293

294-
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
295-
workspaceId: 'ws-1',
296-
chatId: body.id,
297-
type: 'created',
298-
})
294+
expect(mockPublishStatusChanged).toHaveBeenCalledWith(
295+
expect.objectContaining({ workspaceId: 'ws-1' }),
296+
{
297+
chatId: body.id,
298+
type: 'created',
299+
}
300+
)
299301
expect(mockCaptureServerEvent).toHaveBeenCalledWith(
300302
'user-1',
301303
'task_forked',

‎apps/sim/app/api/mothership/chats/[chatId]/fork/route.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
rewriteMessageFileRefs,
2020
rewriteResourceFileRefs,
2121
} from '@/lib/copilot/chat/rewrite-file-references'
22-
import { chatPubSub } from '@/lib/copilot/chat-status'
22+
import { publishChatStatusChanged } from '@/lib/copilot/chat-status'
2323
import { fetchGo } from '@/lib/copilot/request/go/fetch'
2424
import {
2525
authenticateCopilotRequestSessionOnly,
@@ -267,13 +267,7 @@ export const POST = withRouteHandler(
267267
logger.warn('Failed to fork copilot-service conversation, skipping', { err })
268268
}
269269

270-
if (newChat.workspaceId) {
271-
chatPubSub?.publishStatusChanged({
272-
workspaceId: newChat.workspaceId,
273-
chatId: newId,
274-
type: 'created',
275-
})
276-
}
270+
publishChatStatusChanged({ ...parent, userId }, { chatId: newId, type: 'created' })
277271

278272
captureServerEvent(
279273
userId,

‎apps/sim/app/api/mothership/chats/[chatId]/restore/route.test.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
2626
}))
2727

2828
vi.mock('@/lib/copilot/chat-status', () => ({
29-
chatPubSub: { publishStatusChanged: mockPublishStatusChanged },
29+
publishChatStatusChanged: mockPublishStatusChanged,
3030
}))
3131

3232
vi.mock('@/lib/posthog/server', () => ({
@@ -101,11 +101,13 @@ describe('POST /api/mothership/chats/[chatId]/restore', () => {
101101
updatedAt: expect.any(Date),
102102
lastSeenAt: expect.any(Date),
103103
})
104-
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
105-
workspaceId: 'workspace-1',
106-
chatId: 'chat-1',
107-
type: 'created',
108-
})
104+
expect(mockPublishStatusChanged).toHaveBeenCalledWith(
105+
expect.objectContaining({ workspaceId: 'workspace-1' }),
106+
{
107+
chatId: 'chat-1',
108+
type: 'created',
109+
}
110+
)
109111
})
110112

111113
it('returns 404 when the chat is restored concurrently before the update lands', async () => {

‎apps/sim/app/api/mothership/chats/[chatId]/restore/route.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { type NextRequest, NextResponse } from 'next/server'
66
import { restoreMothershipChatContract } from '@/lib/api/contracts/mothership-chats'
77
import { parseRequest } from '@/lib/api/server'
88
import { authorizeOrganizationChat } from '@/lib/copilot/chat/organization-chats'
9-
import { chatPubSub } from '@/lib/copilot/chat-status'
9+
import { publishChatStatusChanged } from '@/lib/copilot/chat-status'
1010
import {
1111
authenticateCopilotRequestSessionOnly,
1212
createForbiddenResponse,
@@ -95,12 +95,8 @@ export const POST = withRouteHandler(
9595
return NextResponse.json({ success: false, error: 'Chat not found' }, { status: 404 })
9696
}
9797

98+
publishChatStatusChanged({ ...restoredChat, userId }, { chatId, type: 'created' })
9899
if (restoredChat.workspaceId) {
99-
chatPubSub?.publishStatusChanged({
100-
workspaceId: restoredChat.workspaceId,
101-
chatId,
102-
type: 'created',
103-
})
104100
captureServerEvent(
105101
userId,
106102
'task_restored',

‎apps/sim/app/api/mothership/chats/[chatId]/route.test.ts‎

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ vi.mock('@/lib/copilot/chat/persisted-message', () => ({
5959
}))
6060

6161
vi.mock('@/lib/copilot/chat-status', () => ({
62-
chatPubSub: { publishStatusChanged: vi.fn() },
62+
publishChatStatusChanged: vi.fn(),
6363
}))
6464

6565
vi.mock('@/lib/billing/storage', () => ({
@@ -71,7 +71,8 @@ vi.mock('@/lib/posthog/server', () => ({
7171
captureServerEvent: vi.fn(),
7272
}))
7373

74-
import { DELETE, GET } from '@/app/api/mothership/chats/[chatId]/route'
74+
import { publishChatStatusChanged } from '@/lib/copilot/chat-status'
75+
import { DELETE, GET, PATCH } from '@/app/api/mothership/chats/[chatId]/route'
7576

7677
function makeContext(chatId: string) {
7778
return { params: Promise.resolve({ chatId }) }
@@ -307,3 +308,67 @@ describe('DELETE /api/mothership/chats/[chatId]', () => {
307308
expect(mockDecrementStorageUsageForBillingContextInTx).not.toHaveBeenCalled()
308309
})
309310
})
311+
312+
describe('organization chat mutations publish private owner updates', () => {
313+
beforeEach(() => {
314+
vi.clearAllMocks()
315+
resetDbChainMock()
316+
copilotHttpMockFns.mockAuthenticateCopilotRequestSessionOnly.mockResolvedValue({
317+
userId: 'user-1',
318+
isAuthenticated: true,
319+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
320+
})
321+
mockGetAccessibleCopilotChat.mockResolvedValue({
322+
id: 'chat-1',
323+
type: 'mothership',
324+
organizationId: 'org-1',
325+
userId: 'user-1',
326+
})
327+
dbChainMockFns.returning.mockResolvedValue([
328+
{ id: 'chat-1', workspaceId: null, organizationId: 'org-1' },
329+
])
330+
})
331+
332+
it.each([{ title: 'New title' }, { pinned: true }, { isUnread: true }, { isUnread: false }])(
333+
'publishes after updating %j',
334+
async (body) => {
335+
const response = await PATCH(
336+
new NextRequest('http://localhost/api/mothership/chats/chat-1', {
337+
method: 'PATCH',
338+
body: JSON.stringify(body),
339+
}),
340+
makeContext('chat-1')
341+
)
342+
expect(response.status).toBe(200)
343+
expect(publishChatStatusChanged).toHaveBeenCalledWith(
344+
expect.objectContaining({ organizationId: 'org-1', userId: 'user-1' }),
345+
{ chatId: 'chat-1', type: 'title' in body ? 'renamed' : 'updated' }
346+
)
347+
}
348+
)
349+
350+
it('publishes deletion under the same owner', async () => {
351+
const response = await DELETE(
352+
new NextRequest('http://localhost/api/mothership/chats/chat-1', { method: 'DELETE' }),
353+
makeContext('chat-1')
354+
)
355+
expect(response.status).toBe(200)
356+
expect(publishChatStatusChanged).toHaveBeenCalledWith(
357+
expect.objectContaining({ organizationId: 'org-1', userId: 'user-1' }),
358+
{ chatId: 'chat-1', type: 'deleted' }
359+
)
360+
})
361+
362+
it('does not publish if a concurrent deletion leaves no updated row', async () => {
363+
dbChainMockFns.returning.mockResolvedValueOnce([])
364+
const response = await PATCH(
365+
new NextRequest('http://localhost/api/mothership/chats/chat-1', {
366+
method: 'PATCH',
367+
body: JSON.stringify({ pinned: true }),
368+
}),
369+
makeContext('chat-1')
370+
)
371+
expect(response.status).toBe(404)
372+
expect(publishChatStatusChanged).not.toHaveBeenCalled()
373+
})
374+
})

‎apps/sim/app/api/mothership/chats/[chatId]/route.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
} from '@/lib/copilot/chat/lifecycle'
1919
import { normalizeMessage } from '@/lib/copilot/chat/persisted-message'
2020
import { reconcileChatStreamMarkers } from '@/lib/copilot/chat/stream-liveness'
21-
import { chatPubSub } from '@/lib/copilot/chat-status'
21+
import { publishChatStatusChanged } from '@/lib/copilot/chat-status'
2222
import {
2323
authenticateCopilotRequestSessionOnly,
2424
createInternalServerErrorResponse,
@@ -199,19 +199,22 @@ export const PATCH = withRouteHandler(
199199
.returning({
200200
id: copilotChats.id,
201201
workspaceId: copilotChats.workspaceId,
202+
organizationId: copilotChats.organizationId,
202203
})
203204

204205
if (!updatedChat) {
205206
return NextResponse.json({ success: false, error: 'Chat not found' }, { status: 404 })
206207
}
207208

209+
publishChatStatusChanged(
210+
{ ...updatedChat, userId },
211+
{
212+
chatId,
213+
type: title !== undefined ? 'renamed' : 'updated',
214+
}
215+
)
208216
if (updatedChat.workspaceId) {
209217
if (title !== undefined) {
210-
chatPubSub?.publishStatusChanged({
211-
workspaceId: updatedChat.workspaceId,
212-
chatId,
213-
type: 'renamed',
214-
})
215218
captureServerEvent(
216219
userId,
217220
'task_renamed',
@@ -281,18 +284,15 @@ export const DELETE = withRouteHandler(
281284
)
282285
.returning({
283286
workspaceId: copilotChats.workspaceId,
287+
organizationId: copilotChats.organizationId,
284288
})
285289

286290
if (!deletedChat) {
287291
return NextResponse.json({ success: false, error: 'Chat not found' }, { status: 404 })
288292
}
289293

294+
publishChatStatusChanged({ ...deletedChat, userId }, { chatId, type: 'deleted' })
290295
if (deletedChat.workspaceId) {
291-
chatPubSub?.publishStatusChanged({
292-
workspaceId: deletedChat.workspaceId,
293-
chatId,
294-
type: 'deleted',
295-
})
296296
captureServerEvent(
297297
userId,
298298
'task_deleted',

‎apps/sim/app/api/mothership/chats/read/route.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ vi.mock('@/lib/copilot/chat/lifecycle', () => ({
1717
getAccessibleCopilotChatAuth: mockGetAccessibleChat,
1818
}))
1919

20+
vi.mock('@/lib/copilot/chat-status', () => ({ publishChatStatusChanged: vi.fn() }))
21+
22+
import { publishChatStatusChanged } from '@/lib/copilot/chat-status'
2023
import { POST } from '@/app/api/mothership/chats/read/route'
2124

2225
function createRequest() {
@@ -67,6 +70,22 @@ describe('POST /api/mothership/chats/read', () => {
6770
)
6871
})
6972

73+
it('broadcasts only a changed read marker, avoiding read/refetch loops', async () => {
74+
mockGetAccessibleChat.mockResolvedValue({
75+
id: 'chat-1',
76+
type: 'mothership',
77+
organizationId: 'org-1',
78+
userId: 'user-1',
79+
})
80+
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'chat-1' }]).mockResolvedValueOnce([])
81+
await POST(createRequest())
82+
await POST(createRequest())
83+
expect(publishChatStatusChanged).toHaveBeenCalledExactlyOnceWith(
84+
expect.objectContaining({ organizationId: 'org-1', userId: 'user-1' }),
85+
{ chatId: 'chat-1', type: 'updated' }
86+
)
87+
})
88+
7089
it('does not update a chat the caller can no longer access', async () => {
7190
mockGetAccessibleChat.mockResolvedValueOnce(null)
7291
const res = await POST(createRequest())

0 commit comments

Comments
 (0)