Skip to content

Commit a89b158

Browse files
authored
fix(organizations): align sidebar loading and chat actions (#7691)
* fix(organizations): align sidebar loading and chat actions * fix(organizations): preserve chat context during pagination
1 parent a2f1ac7 commit a89b158

14 files changed

Lines changed: 840 additions & 207 deletions

File tree

apps/sim/app/o/[organizationId]/components/organization-sidebar/components/chats-section/chats-section.test.tsx

Lines changed: 148 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
88
import type { OrganizationChat } from '@/app/o/[organizationId]/components/organization-sidebar/hooks'
99

1010
const hoverState = vi.hoisted(() => ({ isOpen: false }))
11+
const mockRequestJson = vi.hoisted(() => vi.fn())
12+
13+
vi.mock('@/lib/api/client/request', () => ({ requestJson: mockRequestJson }))
1114

1215
vi.mock('next/link', () => ({
1316
default: ({
@@ -25,7 +28,7 @@ vi.mock('next/link', () => ({
2528
</a>
2629
),
2730
}))
28-
vi.mock('@/app/workspace/[workspaceId]/w/components/sidebar/hooks', () => ({
31+
vi.mock('@/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-hover-menu', () => ({
2932
useHoverMenu: () => ({
3033
isOpen: hoverState.isOpen,
3134
open: vi.fn(),
@@ -37,6 +40,7 @@ vi.mock('@/app/workspace/[workspaceId]/w/components/sidebar/hooks', () => ({
3740
}))
3841

3942
import { ChatsSection } from '@/app/o/[organizationId]/components/organization-sidebar/components/chats-section/chats-section'
43+
import { mothershipChatKeys } from '@/hooks/queries/mothership-chats'
4044

4145
const CHATS: OrganizationChat[] = Array.from({ length: 8 }, (_, index) => ({
4246
id: `chat-${index + 1}`,
@@ -59,6 +63,8 @@ beforeEach(() => {
5963
disconnect() {}
6064
}
6165
)
66+
vi.clearAllMocks()
67+
mockRequestJson.mockResolvedValue({ success: true })
6268
hoverState.isOpen = false
6369
queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
6470
prefetchQuery = vi.spyOn(queryClient, 'prefetchQuery').mockResolvedValue()
@@ -83,9 +89,7 @@ async function render(props: Partial<Parameters<typeof ChatsSection>[0]> = {}) {
8389
isLoading={false}
8490
isCollapsed={false}
8591
pathname={null}
86-
menuOpenHref={null}
87-
onContextMenu={() => {}}
88-
onMoreClick={() => {}}
92+
organizationId='org-1'
8993
{...props}
9094
/>
9195
</QueryClientProvider>
@@ -94,11 +98,19 @@ async function render(props: Partial<Parameters<typeof ChatsSection>[0]> = {}) {
9498
}
9599

96100
describe('ChatsSection', () => {
97-
it('lists every chat with no paging control', async () => {
101+
it('shows five chats with the workspace-style See more and See less controls', async () => {
98102
await render()
99-
103+
expect(container.querySelectorAll('a[href^="/o/org-1/chat/"]')).toHaveLength(5)
104+
const more = Array.from(container.querySelectorAll('button')).find(
105+
(button) => button.textContent === 'See more'
106+
)!
107+
await act(async () => more.click())
100108
expect(container.querySelectorAll('a[href^="/o/org-1/chat/"]')).toHaveLength(8)
101-
expect(container.textContent).not.toContain('See more')
109+
const less = Array.from(container.querySelectorAll('button')).find(
110+
(button) => button.textContent === 'See less'
111+
)!
112+
await act(async () => less.click())
113+
expect(container.querySelectorAll('a[href^="/o/org-1/chat/"]')).toHaveLength(5)
102114
})
103115

104116
it('marks the chat on the current route active', async () => {
@@ -110,17 +122,138 @@ describe('ChatsSection', () => {
110122
expect(other?.className).not.toContain('surface-active')
111123
})
112124

113-
it('reports the row href when its options button is pressed', async () => {
114-
const onMoreClick = vi.fn()
115-
await render({ onMoreClick })
125+
it('keeps a bookmarked chat visible when collapsing expanded history', async () => {
126+
await render({ pathname: CHATS[5].href })
127+
expect(container.querySelectorAll('a[href^="/o/org-1/chat/"]')).toHaveLength(6)
128+
expect(container.querySelector(`a[href="${CHATS[5].href}"]`)?.className).toContain(
129+
'surface-active'
130+
)
131+
const more = Array.from(container.querySelectorAll('button')).find(
132+
(button) => button.textContent === 'See more'
133+
)!
134+
await act(async () => more.click())
135+
expect(container.querySelectorAll('a[href^="/o/org-1/chat/"]')).toHaveLength(8)
136+
const less = Array.from(container.querySelectorAll('button')).find(
137+
(button) => button.textContent === 'See less'
138+
)!
139+
await act(async () => less.click())
140+
expect(container.querySelectorAll('a[href^="/o/org-1/chat/"]')).toHaveLength(6)
141+
expect(container.querySelector(`a[href="${CHATS[5].href}"]`)).not.toBeNull()
142+
})
143+
144+
it('derives the visible range from the route without retaining automatic expansion', async () => {
145+
await render({ pathname: CHATS[7].href })
146+
expect(container.querySelectorAll('a[href^="/o/org-1/chat/"]')).toHaveLength(8)
147+
expect(container.textContent).not.toContain('See more')
148+
expect(container.textContent).not.toContain('See less')
149+
150+
await render({ pathname: null })
151+
expect(container.querySelectorAll('a[href^="/o/org-1/chat/"]')).toHaveLength(5)
152+
expect(container.textContent).toContain('See more')
153+
})
154+
155+
it.each([false, true])('renames via the options menu with collapsed=%s', async (isCollapsed) => {
156+
hoverState.isOpen = isCollapsed
157+
await render({ isCollapsed })
158+
const button =
159+
document.body.querySelector<HTMLButtonElement>(
160+
'a[href="/o/org-1/chat/chat-2"] button[aria-label="Chat options"]'
161+
) ?? document.body.querySelector<HTMLButtonElement>('[aria-label="Chat options"]')!
162+
await act(async () => button.click())
163+
const rename = Array.from(
164+
document.body.querySelectorAll<HTMLElement>('[role="menuitem"]')
165+
).find((item) => item.textContent === 'Rename')!
166+
expect(rename).toBeDefined()
167+
await act(async () => rename.click())
168+
const input = document.body.querySelector<HTMLInputElement>('input[aria-label^="Rename chat"]')!
169+
expect(input).not.toBeNull()
170+
expect(input.value).toMatch(/^Chat /)
171+
await act(async () => {
172+
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!.call(
173+
input,
174+
'Planning'
175+
)
176+
input.dispatchEvent(new Event('input', { bubbles: true }))
177+
})
178+
await act(async () =>
179+
input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))
180+
)
181+
expect(mockRequestJson).toHaveBeenCalledWith(
182+
expect.objectContaining({ method: 'PATCH' }),
183+
expect.objectContaining({ body: { title: 'Planning' } })
184+
)
185+
expect(document.body.querySelector('input[aria-label^="Rename chat"]')).toBeNull()
186+
})
187+
188+
it('rolls back only the organization list when rename fails', async () => {
189+
const pending = Promise.withResolvers<{ success: boolean }>()
190+
mockRequestJson.mockReturnValueOnce(pending.promise)
191+
const key = mothershipChatKeys.organizationList('org-1')
192+
queryClient.setQueryData(key, [{ id: 'chat-1', name: 'Chat 1' }])
193+
const workspaceKey = mothershipChatKeys.list('workspace-1')
194+
queryClient.setQueryData(workspaceKey, [{ id: 'workspace-chat', name: 'Workspace chat' }])
195+
await render()
196+
await act(async () =>
197+
container.querySelector<HTMLButtonElement>('[aria-label="Chat options"]')!.click()
198+
)
199+
const action = Array.from(
200+
document.body.querySelectorAll<HTMLElement>('[role="menuitem"]')
201+
).find((item) => item.textContent === 'Rename')!
202+
await act(async () => action.click())
203+
const input = document.body.querySelector<HTMLInputElement>('input[aria-label^="Rename chat"]')!
204+
await act(async () => {
205+
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!.call(
206+
input,
207+
'Pending title'
208+
)
209+
input.dispatchEvent(new Event('input', { bubbles: true }))
210+
})
211+
await act(async () =>
212+
input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))
213+
)
214+
expect(queryClient.getQueryData(key)).toEqual([{ id: 'chat-1', name: 'Pending title' }])
215+
await act(async () => pending.reject(new Error('Rename rejected')))
216+
expect(queryClient.getQueryData(key)).toEqual([{ id: 'chat-1', name: 'Chat 1' }])
217+
expect(queryClient.getQueryData(workspaceKey)).toEqual([
218+
{ id: 'workspace-chat', name: 'Workspace chat' },
219+
])
220+
expect(input.value).toBe('Chat 1')
221+
expect(input.disabled).toBe(false)
222+
})
116223

117-
const button = container.querySelector<HTMLButtonElement>(
118-
'a[href="/o/org-1/chat/chat-2"] button[aria-label="Chat options"]'
224+
it('cancels rename on Escape without a mutation', async () => {
225+
await render()
226+
await act(async () =>
227+
container.querySelector<HTMLButtonElement>('[aria-label="Chat options"]')!.click()
119228
)
120-
await act(async () => button?.click())
229+
const rename = Array.from(
230+
document.body.querySelectorAll<HTMLElement>('[role="menuitem"]')
231+
).find((item) => item.textContent === 'Rename')!
232+
await act(async () => rename.click())
233+
const input = document.body.querySelector<HTMLInputElement>('input[aria-label^="Rename chat"]')!
234+
await act(async () =>
235+
input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))
236+
)
237+
expect(mockRequestJson).not.toHaveBeenCalled()
238+
expect(document.body.querySelector('input[aria-label^="Rename chat"]')).toBeNull()
239+
})
121240

122-
expect(onMoreClick).toHaveBeenCalledWith(expect.anything(), '/o/org-1/chat/chat-2')
123-
expect(prefetchQuery).not.toHaveBeenCalled()
241+
it.each([
242+
['Pin', { pinned: true }],
243+
['Mark as unread', { isUnread: true }],
244+
])('offers %s for organization chats', async (label, body) => {
245+
await render()
246+
await act(async () =>
247+
container.querySelector<HTMLButtonElement>('[aria-label="Chat options"]')!.click()
248+
)
249+
const action = Array.from(
250+
document.body.querySelectorAll<HTMLElement>('[role="menuitem"]')
251+
).find((item) => item.textContent === label)!
252+
await act(async () => action.click())
253+
expect(mockRequestJson).toHaveBeenCalledWith(expect.objectContaining({ method: 'PATCH' }), {
254+
params: { chatId: 'chat-1' },
255+
body,
256+
})
124257
})
125258

126259
it.each([false, true])(

0 commit comments

Comments
 (0)