Skip to content

Commit 281ba8c

Browse files
authored
fix(sidebar): simplify organization controls and add logo uploads (#7783)
* fix(sidebar): simplify organization controls and add logo uploads * fix(uploads): guard logo replacements and reclaim retired images
1 parent 2217f71 commit 281ba8c

44 files changed

Lines changed: 27709 additions & 216 deletions

Some content is hidden

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

.github/workflows/test-build.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ jobs:
190190
lib/knowledge/__integration__/search-reference-batching.integration.ts
191191
lib/core/outbox/service.integration.ts
192192
lib/knowledge/__integration__/connector-upload.integration.ts
193+
lib/uploads/contexts/organization-logo/application.integration.ts
193194
194195
test-build:
195196
name: Lint and Test

apps/sim/app/api/files/authorization.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,22 @@ describe('public-context access (profile-pictures / og-images / workspace-logos)
182182
return verifyFileAccess(cloudKey, USER_ID, undefined, context, false, { requireWrite: true })
183183
}
184184

185+
it('allows organization logo reads and denies generic deletes even for the uploader', async () => {
186+
const key = 'organization-logos/org-1/logo.png'
187+
mockGetFileMetadata.mockResolvedValue({ userId: USER_ID })
188+
await expect(verifyFileAccess(key, USER_ID, undefined, 'organization-logos')).resolves.toBe(
189+
true
190+
)
191+
await expect(
192+
verifyFileAccess(key, USER_ID, undefined, 'organization-logos', false, { requireWrite: true })
193+
).resolves.toBe(false)
194+
await expect(
195+
verifyFileAccess(key, USER_ID, undefined, 'general', false, { requireWrite: true })
196+
).resolves.toBe(false)
197+
expect(mockGetFileMetadata).not.toHaveBeenCalled()
198+
expect(mockGetUserEntityPermissions).not.toHaveBeenCalled()
199+
})
200+
185201
it('grants public reads without any ownership check', async () => {
186202
await expect(read('og-images/banner.png', 'og-images')).resolves.toBe(true)
187203
await expect(read('profile-pictures/123-avatar.png', 'profile-pictures')).resolves.toBe(true)

apps/sim/app/api/files/authorization.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ export async function verifyFileAccess(
155155
const requireWrite = options?.requireWrite ?? false
156156
try {
157157
const keyContext = inferContextFromKey(cloudKey)
158+
/** Organization logos are changed only through the organization-authorized upload lifecycle. */
159+
if (keyContext === 'organization-logos') return !requireWrite
158160
if (keyContext === 'knowledge-base') {
159161
return requireWrite
160162
? verifyKBFileWriteAccess(cloudKey, userId)

apps/sim/app/api/files/serve/[...path]/route.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,28 @@ describe('File Serve API Route', () => {
456456
})
457457
})
458458

459+
it('serves organization logos through the existing public asset path', async () => {
460+
mockIsUsingCloudStorage.mockReturnValue(true)
461+
mockInferContextFromKey.mockReturnValue('organization-logos')
462+
const key = 'organization-logos/org-1/upload-1-logo.png'
463+
const response = await GET(new NextRequest(`http://localhost/api/files/serve/s3/${key}`), {
464+
params: Promise.resolve({ path: ['s3', 'organization-logos', 'org-1', 'upload-1-logo.png'] }),
465+
})
466+
expect(response.status).toBe(200)
467+
expect(storageServiceMockFns.mockDownloadFile).toHaveBeenCalledWith({
468+
key,
469+
context: 'organization-logos',
470+
maxBytes: MAX_BUFFERED_TRANSFER_BYTES,
471+
})
472+
expect(mockCreateFileResponse).toHaveBeenCalledWith(
473+
expect.objectContaining({
474+
cacheControl: 'public, max-age=31536000',
475+
})
476+
)
477+
expect(mockVerifyFileAccess).not.toHaveBeenCalled()
478+
expect(mockAuthenticateWorkspaceFile).not.toHaveBeenCalled()
479+
})
480+
459481
it('should return 404 when file not found', async () => {
460482
mockVerifyFileAccess.mockResolvedValue(false)
461483
mockFindLocalFile.mockReturnValue(null)

apps/sim/app/api/files/serve/[...path]/route.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,8 @@ export const GET = withRouteHandler(
238238
const isPublicByKeyPrefix =
239239
cloudKey.startsWith('profile-pictures/') ||
240240
cloudKey.startsWith('og-images/') ||
241-
cloudKey.startsWith('workspace-logos/')
241+
cloudKey.startsWith('workspace-logos/') ||
242+
cloudKey.startsWith('organization-logos/')
242243

243244
if (isPublicByKeyPrefix) {
244245
const context = inferContextFromKey(cloudKey)

apps/sim/app/api/files/uploads/finalizers.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ import { captureServerEvent } from '@/lib/posthog/server'
1111
import { notifyWorkspaceFilesChanged } from '@/lib/realtime/notify'
1212
import { getServeStoragePrefix } from '@/lib/uploads/config'
1313
import { finalizeOrganizationAssistantAttachment } from '@/lib/uploads/contexts/organization-assistant/application'
14+
import {
15+
finalizeOrganizationLogoUpload,
16+
organizationLogoUploadResult,
17+
} from '@/lib/uploads/contexts/organization-logo/application'
1418
import {
1519
getWorkspaceFile,
1620
registerUploadedWorkspaceFile,
@@ -106,6 +110,8 @@ export async function finalizeUploadPurpose({
106110
)
107111
case 'profile_picture':
108112
return { value: storedAssetResult(session, 'profile-pictures') }
113+
case 'organization_logo':
114+
return finalizeOrganizationLogoUpload(principal, session, request)
109115
case 'workspace_logo':
110116
return finalizeWorkspaceLogo(session, actor, request)
111117
case 'mothership_attachment':
@@ -137,6 +143,8 @@ export async function loadCompletedUploadPurpose(
137143
switch (session.purpose) {
138144
case 'workspace_file':
139145
return toV2File(await loadCompletedWorkspaceFileUpload(session))
146+
case 'organization_logo':
147+
return organizationLogoUploadResult(session)
140148
case 'profile_picture':
141149
case 'workspace_logo':
142150
case 'mothership_attachment':

apps/sim/app/api/files/uploads/purposes.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const INTERNAL_UPLOAD_PURPOSES = new Set<InternalUploadPurpose>([
2424
'workspace_file',
2525
'profile_picture',
2626
'workspace_logo',
27+
'organization_logo',
2728
'mothership_attachment',
2829
'execution_attachment',
2930
])
@@ -57,6 +58,11 @@ export async function createPurposeUploadSession(
5758
localOrigin,
5859
})
5960
}
61+
case 'organization_logo':
62+
throw new UploadSessionError(
63+
'validation',
64+
'Organization logos require organization authorization'
65+
)
6066
case 'profile_picture':
6167
return createUploadSession({
6268
purpose: body.purpose,
@@ -121,6 +127,11 @@ export async function reauthorizeUploadPurpose(
121127
case 'mothership_attachment':
122128
await requireWorkspacePermission(userId, requireSessionScope(session.workspaceId), 'write')
123129
return
130+
case 'organization_logo':
131+
throw new UploadSessionError(
132+
'forbidden',
133+
'Organization logos require organization authorization'
134+
)
124135
case 'profile_picture':
125136
return
126137
case 'workspace_logo':

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

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,6 @@
11
'use client'
22

3-
import {
4-
ChipInput,
5-
chipVariants,
6-
cn,
7-
DropdownMenuItem,
8-
Loader,
9-
OverflowText,
10-
Skeleton,
11-
} from '@sim/emcn'
3+
import { chipVariants, cn, DropdownMenuItem, Loader, OverflowText, Skeleton } from '@sim/emcn'
124
import { MoreHorizontal, Pin, Task } from '@sim/emcn/icons'
135
import type { OrganizationChat } from '@/app/o/[organizationId]/components/organization-sidebar/hooks'
146
import { useOrganizationChatActions } from '@/app/o/[organizationId]/components/organization-sidebar/hooks/use-organization-chat-actions'
@@ -18,6 +10,7 @@ import {
1810
CollapsedSidebarMenu,
1911
SidebarSection,
2012
} from '@/app/workspace/[workspaceId]/w/components/sidebar/components'
13+
import { SidebarRenameRow } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-rename-row'
2114
import { ContextMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu'
2215
import { DeleteModal } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/delete-modal/delete-modal'
2316
import {
@@ -187,7 +180,7 @@ export function ChatsSection({
187180
)}
188181
{chats.map((chat) =>
189182
rename.editingId === chat.id ? (
190-
<ChipInput
183+
<SidebarRenameRow
191184
key={chat.id}
192185
ref={rename.inputRef}
193186
aria-label={`Rename chat ${chat.name}`}
@@ -196,8 +189,6 @@ export function ChatsSection({
196189
onKeyDown={rename.handleKeyDown}
197190
onBlur={saveRename}
198191
disabled={rename.isSaving}
199-
maxLength={100}
200-
autoComplete='off'
201192
/>
202193
) : (
203194
<ChatRow
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { ToastProvider } from '@sim/emcn'
6+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
7+
import { createRoot, type Root } from 'react-dom/client'
8+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
9+
10+
const mocks = vi.hoisted(() => ({ upload: vi.fn(), refresh: vi.fn() }))
11+
vi.mock('@/lib/uploads/client/session-upload', () => ({
12+
uploadInternalFileSession: mocks.upload,
13+
}))
14+
vi.mock('next/navigation', () => ({
15+
useRouter: () => ({ refresh: mocks.refresh }),
16+
usePathname: () => '/o/org-1/home',
17+
}))
18+
19+
import { OrganizationHeader } from '@/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header'
20+
import { organizationKeys } from '@/hooks/queries/utils/organization-keys'
21+
22+
const organization = { id: 'org-1', name: 'Design', slug: 'design', logo: null, memberCount: 2 }
23+
let root: Root
24+
let container: HTMLDivElement
25+
let queryClient: QueryClient
26+
27+
beforeEach(() => {
28+
vi.clearAllMocks()
29+
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
30+
vi.stubGlobal(
31+
'ResizeObserver',
32+
class {
33+
observe() {}
34+
unobserve() {}
35+
disconnect() {}
36+
}
37+
)
38+
mocks.upload.mockResolvedValue({ path: '/api/files/serve/organization-logos/logo.png' })
39+
queryClient = new QueryClient({ defaultOptions: { mutations: { retry: false } } })
40+
container = document.createElement('div')
41+
document.body.append(container)
42+
root = createRoot(container)
43+
})
44+
45+
afterEach(async () => {
46+
await act(async () => root.unmount())
47+
container.remove()
48+
queryClient.clear()
49+
vi.unstubAllGlobals()
50+
})
51+
52+
async function render(canEditLogo = true) {
53+
await act(async () => {
54+
root.render(
55+
<QueryClientProvider client={queryClient}>
56+
<ToastProvider>
57+
<OrganizationHeader
58+
organization={organization}
59+
canEditLogo={canEditLogo}
60+
isCollapsed={false}
61+
onExpandSidebar={vi.fn()}
62+
/>
63+
</ToastProvider>
64+
</QueryClientProvider>
65+
)
66+
})
67+
}
68+
69+
async function openMenu() {
70+
await act(async () => {
71+
container
72+
.querySelector('[aria-label="Organization menu"]')!
73+
.dispatchEvent(new MouseEvent('pointerdown', { bubbles: true, button: 0 }))
74+
})
75+
}
76+
77+
function menuItem(name: string) {
78+
return Array.from(document.querySelectorAll<HTMLElement>('[role="menuitem"]')).find(
79+
(item) => item.textContent === name
80+
)
81+
}
82+
83+
async function pickFile(file: File) {
84+
const input = container.querySelector<HTMLInputElement>('input[type="file"]')!
85+
Object.defineProperty(input, 'files', { configurable: true, value: [file] })
86+
await act(async () => input.dispatchEvent(new Event('change', { bubbles: true })))
87+
}
88+
89+
describe('OrganizationHeader logo upload', () => {
90+
it('opens the same native file picker from the admin menu', async () => {
91+
await render()
92+
await openMenu()
93+
const input = container.querySelector<HTMLInputElement>('input[type="file"]')!
94+
const click = vi.spyOn(input, 'click').mockImplementation(() => {})
95+
expect(input.accept).toContain('image/png')
96+
await act(async () => menuItem('Upload logo')!.click())
97+
expect(click).toHaveBeenCalledOnce()
98+
})
99+
100+
it('does not offer logo changes to members', async () => {
101+
await render(false)
102+
await openMenu()
103+
expect(menuItem('Upload logo')).toBeUndefined()
104+
expect(container.querySelector('input[type="file"]')).toBeNull()
105+
})
106+
107+
it('uploads under the organization scope and refreshes its identity after success', async () => {
108+
await render()
109+
const invalidate = vi.spyOn(queryClient, 'invalidateQueries')
110+
const file = new File(['image'], 'logo.png', { type: 'image/png' })
111+
await pickFile(file)
112+
expect(mocks.upload).toHaveBeenCalledWith({
113+
purpose: 'organization_logo',
114+
organizationId: organization.id,
115+
file,
116+
})
117+
expect(invalidate).toHaveBeenCalledWith({ queryKey: organizationKeys.detail('org-1') })
118+
expect(invalidate).toHaveBeenCalledWith({ queryKey: organizationKeys.lists() })
119+
expect(mocks.refresh).toHaveBeenCalledOnce()
120+
})
121+
122+
it('rejects unsupported files before uploading', async () => {
123+
await render()
124+
await pickFile(new File(['text'], 'notes.txt', { type: 'text/plain' }))
125+
expect(mocks.upload).not.toHaveBeenCalled()
126+
expect(mocks.refresh).not.toHaveBeenCalled()
127+
expect(document.body.textContent).toContain('not a supported image format')
128+
})
129+
130+
it('keeps the saved identity when upload fails and allows retrying the same file', async () => {
131+
const file = new File(['image'], 'logo.png', { type: 'image/png' })
132+
mocks.upload.mockRejectedValueOnce(new Error('Upload failed'))
133+
await render()
134+
await pickFile(file)
135+
expect(mocks.refresh).not.toHaveBeenCalled()
136+
expect(document.body.textContent).toContain('Upload failed')
137+
expect(container.querySelector<HTMLInputElement>('input[type="file"]')!.value).toBe('')
138+
await pickFile(file)
139+
expect(mocks.upload).toHaveBeenCalledTimes(2)
140+
expect(mocks.refresh).toHaveBeenCalledOnce()
141+
})
142+
})

0 commit comments

Comments
 (0)