Skip to content

Commit c77300f

Browse files
authored
fix(connectors): resolve SharePoint folder paths against the right document library (#6026)
Folder-scoped SharePoint connectors failed with "Folder not found" for folders that exist and are readable with the same credential, leaving whole-library sync as the only option. - resolve the target drive explicitly and thread it through listing, download and hydration, which previously hardcoded the site default - resolve folder paths in layers: byte-exact addressing first (unchanged), then a leading document-library name, then a normalized children walk that recovers names carrying non-breaking or invisible whitespace - accept a folder URL from the browser address bar - report the site, library, attempted path and existing folder names on failure instead of a bare "Folder not found" - document the expected folder path format in the connector schema
1 parent a957fa4 commit c77300f

3 files changed

Lines changed: 825 additions & 120 deletions

File tree

apps/sim/connectors/sharepoint/meta.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ export const sharepointConnectorMeta: ConnectorMeta = {
2222
id: 'folderPath',
2323
title: 'Folder Path',
2424
type: 'short-input',
25-
placeholder: 'e.g. Documents/Reports (optional, defaults to root)',
25+
placeholder: 'e.g. Reports/2026 (optional, defaults to the whole library)',
26+
description:
27+
'Path relative to the document library root — omit a leading "Documents" or "Shared Documents". To target a different library, start the path with that library\'s name. You can also paste the folder URL from your browser\'s address bar.',
2628
required: false,
2729
},
2830
{
Lines changed: 343 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,343 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { mockFetchWithRetry } = vi.hoisted(() => ({ mockFetchWithRetry: vi.fn() }))
7+
8+
vi.mock('@/lib/knowledge/documents/utils', () => ({
9+
fetchWithRetry: mockFetchWithRetry,
10+
VALIDATE_RETRY_OPTIONS: {},
11+
}))
12+
vi.mock('@/components/icons', () => ({ MicrosoftSharepointIcon: () => null }))
13+
14+
import {
15+
normalizeSegment,
16+
resolveFolderTarget,
17+
serverRelativePathFromUrl,
18+
} from '@/connectors/sharepoint/sharepoint'
19+
20+
const GRAPH = 'https://graph.microsoft.com/v1.0'
21+
const SITE_ID = 'contoso.sharepoint.com,site-guid,web-guid'
22+
const SITE_URL = 'contoso.sharepoint.com'
23+
const DEFAULT_DRIVE_ID = 'b!default'
24+
const POLICIES_DRIVE_ID = 'b!policies'
25+
26+
interface GraphRoute {
27+
status?: number
28+
body?: unknown
29+
}
30+
31+
/** Folder-shaped drive item for children listings. */
32+
function folder(id: string, name: string) {
33+
return { id, name, folder: { childCount: 0 } }
34+
}
35+
36+
/**
37+
* Installs a URL-keyed fake Graph. Any URL without a route replies 404, which is
38+
* what makes the "falls through to the next layer" assertions meaningful.
39+
*/
40+
function mockGraph(routes: Record<string, GraphRoute>) {
41+
const requested: string[] = []
42+
mockFetchWithRetry.mockImplementation(async (url: string) => {
43+
requested.push(url)
44+
const route = routes[url] ?? { status: 404 }
45+
const status = route.status ?? 200
46+
return {
47+
ok: status >= 200 && status < 300,
48+
status,
49+
json: async () => route.body,
50+
text: async () => JSON.stringify(route.body ?? {}),
51+
} as unknown as Response
52+
})
53+
return requested
54+
}
55+
56+
const defaultDriveRoute = {
57+
[`${GRAPH}/sites/${SITE_ID}/drive?$select=id,name,webUrl`]: {
58+
body: {
59+
id: DEFAULT_DRIVE_ID,
60+
name: 'Documents',
61+
webUrl: 'https://contoso.sharepoint.com/Shared%20Documents',
62+
},
63+
},
64+
}
65+
66+
const sitesDrivesRoute = {
67+
[`${GRAPH}/sites/${SITE_ID}/drives?$select=id,name,webUrl`]: {
68+
body: {
69+
value: [
70+
{
71+
id: DEFAULT_DRIVE_ID,
72+
name: 'Documents',
73+
webUrl: 'https://contoso.sharepoint.com/Shared%20Documents',
74+
},
75+
{
76+
id: POLICIES_DRIVE_ID,
77+
name: 'Policies',
78+
webUrl: 'https://contoso.sharepoint.com/Policies',
79+
},
80+
],
81+
},
82+
},
83+
}
84+
85+
function rootChildren(driveId: string, items: unknown[]) {
86+
return {
87+
[`${GRAPH}/drives/${driveId}/root/children?$top=200&$select=id,name,folder`]: {
88+
body: { value: items },
89+
},
90+
}
91+
}
92+
93+
function resolve(folderPath?: string) {
94+
return resolveFolderTarget('token', SITE_ID, SITE_URL, 'Contoso', folderPath)
95+
}
96+
97+
beforeEach(() => {
98+
vi.clearAllMocks()
99+
})
100+
101+
describe('resolveFolderTarget', () => {
102+
it('returns the default library root when no folder path is configured', async () => {
103+
const requested = mockGraph({ ...defaultDriveRoute })
104+
105+
await expect(resolve(undefined)).resolves.toEqual({
106+
driveId: DEFAULT_DRIVE_ID,
107+
driveName: 'Documents',
108+
})
109+
expect(requested.some((url) => url.includes('root:'))).toBe(false)
110+
})
111+
112+
it('resolves a top-level folder by exact path against the default library', async () => {
113+
const requested = mockGraph({
114+
...defaultDriveRoute,
115+
[`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/00%20IWW%20Library`]: {
116+
body: folder('folder-1', '00 IWW Library'),
117+
},
118+
})
119+
120+
await expect(resolve('00 IWW Library')).resolves.toEqual({
121+
driveId: DEFAULT_DRIVE_ID,
122+
driveName: 'Documents',
123+
folderId: 'folder-1',
124+
})
125+
expect(requested).toContain(`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/00%20IWW%20Library`)
126+
})
127+
128+
it('ignores leading and trailing slashes', async () => {
129+
mockGraph({
130+
...defaultDriveRoute,
131+
[`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/00%20IWW%20Library`]: {
132+
body: folder('folder-1', '00 IWW Library'),
133+
},
134+
})
135+
136+
await expect(resolve('/00 IWW Library/')).resolves.toMatchObject({ folderId: 'folder-1' })
137+
})
138+
139+
it('resolves a nested folder', async () => {
140+
mockGraph({
141+
...defaultDriveRoute,
142+
[`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/00%20IWW%20Library/Templates`]: {
143+
body: folder('folder-2', 'Templates'),
144+
},
145+
})
146+
147+
await expect(resolve('00 IWW Library/Templates')).resolves.toMatchObject({
148+
folderId: 'folder-2',
149+
})
150+
})
151+
152+
it('strips a leading document-library name that is not a real folder', async () => {
153+
mockGraph({
154+
...defaultDriveRoute,
155+
...sitesDrivesRoute,
156+
[`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/00%20IWW%20Library`]: {
157+
body: folder('folder-1', '00 IWW Library'),
158+
},
159+
})
160+
161+
await expect(resolve('Shared Documents/00 IWW Library')).resolves.toEqual({
162+
driveId: DEFAULT_DRIVE_ID,
163+
driveName: 'Documents',
164+
folderId: 'folder-1',
165+
})
166+
})
167+
168+
it('prefers a real folder named "Documents" over the library-name interpretation', async () => {
169+
mockGraph({
170+
...defaultDriveRoute,
171+
...sitesDrivesRoute,
172+
[`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/Documents/Reports`]: {
173+
body: folder('real-nested', 'Reports'),
174+
},
175+
[`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/Reports`]: {
176+
body: folder('wrong-one', 'Reports'),
177+
},
178+
})
179+
180+
await expect(resolve('Documents/Reports')).resolves.toMatchObject({
181+
folderId: 'real-nested',
182+
})
183+
})
184+
185+
it('resolves a folder in a non-default document library', async () => {
186+
mockGraph({
187+
...defaultDriveRoute,
188+
...sitesDrivesRoute,
189+
[`${GRAPH}/drives/${POLICIES_DRIVE_ID}/root:/HR`]: { body: folder('hr-1', 'HR') },
190+
})
191+
192+
await expect(resolve('Policies/HR')).resolves.toEqual({
193+
driveId: POLICIES_DRIVE_ID,
194+
driveName: 'Policies',
195+
folderId: 'hr-1',
196+
})
197+
})
198+
199+
it('resolves a bare non-default library name to that library root', async () => {
200+
mockGraph({ ...defaultDriveRoute, ...sitesDrivesRoute })
201+
202+
await expect(resolve('Policies')).resolves.toEqual({
203+
driveId: POLICIES_DRIVE_ID,
204+
driveName: 'Policies',
205+
})
206+
})
207+
208+
it('recovers a folder whose real name contains a non-breaking space', async () => {
209+
mockGraph({
210+
...defaultDriveRoute,
211+
...sitesDrivesRoute,
212+
...rootChildren(DEFAULT_DRIVE_ID, [
213+
folder('other', 'Archive'),
214+
folder('folder-1', '00\u00a0IWW Library'),
215+
]),
216+
})
217+
218+
await expect(resolve('00 IWW Library')).resolves.toMatchObject({ folderId: 'folder-1' })
219+
})
220+
221+
it('recovers a folder that differs only by case', async () => {
222+
mockGraph({
223+
...defaultDriveRoute,
224+
...sitesDrivesRoute,
225+
...rootChildren(DEFAULT_DRIVE_ID, [folder('folder-1', '00 iww library')]),
226+
})
227+
228+
await expect(resolve('00 IWW LIBRARY')).resolves.toMatchObject({ folderId: 'folder-1' })
229+
})
230+
231+
it('refuses to guess when two sibling folders normalize identically', async () => {
232+
mockGraph({
233+
...defaultDriveRoute,
234+
...sitesDrivesRoute,
235+
...rootChildren(DEFAULT_DRIVE_ID, [
236+
folder('a', '00 IWW Library'),
237+
folder('b', '00\u00a0IWW Library'),
238+
]),
239+
})
240+
241+
await expect(resolve('00 IWW Library')).rejects.toThrow(/matches more than one folder/)
242+
})
243+
244+
it('rejects a path that resolves to a file', async () => {
245+
mockGraph({
246+
...defaultDriveRoute,
247+
[`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/notes.txt`]: {
248+
body: { id: 'f1', name: 'notes.txt', file: { mimeType: 'text/plain' } },
249+
},
250+
})
251+
252+
await expect(resolve('notes.txt')).rejects.toThrow(/is not a folder/)
253+
})
254+
255+
it('reports the site, library, path and existing folders when nothing matches', async () => {
256+
mockGraph({
257+
...defaultDriveRoute,
258+
...sitesDrivesRoute,
259+
...rootChildren(DEFAULT_DRIVE_ID, [folder('a', 'Archive'), folder('b', 'Reports')]),
260+
})
261+
262+
await expect(resolve('00 IWW Library')).rejects.toThrow(
263+
/Folder not found: "00 IWW Library"[\s\S]*Contoso[\s\S]*Documents[\s\S]*"Archive", "Reports"/
264+
)
265+
})
266+
267+
it('surfaces a failure to open the default library rather than reporting not-found', async () => {
268+
mockGraph({
269+
[`${GRAPH}/sites/${SITE_ID}/drive?$select=id,name,webUrl`]: { status: 403 },
270+
})
271+
272+
await expect(resolve('00 IWW Library')).rejects.toThrow(
273+
/Failed to open the default document library/
274+
)
275+
})
276+
277+
it('accepts an address-bar folder URL carrying the path in the id parameter', async () => {
278+
mockGraph({
279+
...defaultDriveRoute,
280+
...sitesDrivesRoute,
281+
[`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/root:/00%20IWW%20Library`]: {
282+
body: folder('folder-1', '00 IWW Library'),
283+
},
284+
})
285+
286+
const url =
287+
'https://contoso.sharepoint.com/Shared%20Documents/Forms/AllItems.aspx' +
288+
'?id=%2FShared%20Documents%2F00%20IWW%20Library&viewid=abc'
289+
290+
await expect(resolve(url)).resolves.toMatchObject({ folderId: 'folder-1' })
291+
})
292+
293+
it('rejects a tokenized sharing link with actionable guidance', async () => {
294+
mockGraph({ ...defaultDriveRoute })
295+
296+
await expect(resolve('https://contoso.sharepoint.com/:f:/s/hr/Ei4xAbC?e=xyz')).rejects.toThrow(
297+
/address bar/
298+
)
299+
})
300+
})
301+
302+
describe('serverRelativePathFromUrl', () => {
303+
it('strips the site prefix from a site-scoped URL', () => {
304+
expect(
305+
serverRelativePathFromUrl(
306+
'https://contoso.sharepoint.com/sites/hr/Shared%20Documents/Reports',
307+
'contoso.sharepoint.com/sites/hr'
308+
)
309+
).toEqual(['Shared Documents', 'Reports'])
310+
})
311+
312+
it('drops the Forms view suffix', () => {
313+
expect(
314+
serverRelativePathFromUrl(
315+
'https://contoso.sharepoint.com/Shared%20Documents/Forms/AllItems.aspx',
316+
'contoso.sharepoint.com'
317+
)
318+
).toEqual(['Shared Documents'])
319+
})
320+
321+
it('returns null for a tokenized sharing link', () => {
322+
expect(
323+
serverRelativePathFromUrl(
324+
'https://contoso.sharepoint.com/:f:/s/hr/Ei4xAbC',
325+
'contoso.sharepoint.com'
326+
)
327+
).toBeNull()
328+
})
329+
})
330+
331+
describe('normalizeSegment', () => {
332+
it('folds non-breaking spaces, repeated whitespace and case', () => {
333+
expect(normalizeSegment('00\u00a0IWW LIBRARY ')).toBe('00 iww library')
334+
})
335+
336+
it('removes zero-width characters', () => {
337+
expect(normalizeSegment('Report\u200bs')).toBe('reports')
338+
})
339+
340+
it('leaves an ordinary name unchanged apart from case', () => {
341+
expect(normalizeSegment('Reports')).toBe('reports')
342+
})
343+
})

0 commit comments

Comments
 (0)