Skip to content

Commit dc94879

Browse files
authored
fix(connectors): attribute SharePoint not-found errors to the matched library (#6029)
* fix(connectors): attribute SharePoint not-found errors to the matched library When the first path segment named a real non-default document library but the remainder did not resolve there, the failure message described a search of the default library over the full original path, and advised stripping a library prefix the user had supplied correctly. Report against the library that was matched, over the remainder that was actually searched, and only suggest omitting a leading library name when the default library really was the one searched. * fix(connectors): key the library-prefix hint on the library actually searched Deriving the flag from `!libraryMatch` suppressed the hint when the path named the default library itself ("Documents/Reports"), which is exactly the case the hint exists for. Key it on whether the reported drive is the default library.
1 parent 3d72ab3 commit dc94879

2 files changed

Lines changed: 57 additions & 5 deletions

File tree

apps/sim/connectors/sharepoint/sharepoint.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,39 @@ describe('resolveFolderTarget', () => {
264264
)
265265
})
266266

267+
it('blames the matched library, not the default one, when its remainder is wrong', async () => {
268+
mockGraph({
269+
...defaultDriveRoute,
270+
...sitesDrivesRoute,
271+
...rootChildren(DEFAULT_DRIVE_ID, [folder('d1', 'Archive')]),
272+
...rootChildren(POLICIES_DRIVE_ID, [folder('p1', 'Onboarding')]),
273+
})
274+
275+
const error = await resolve('Policies/HR').catch((e: Error) => e)
276+
277+
expect(error).toBeInstanceOf(Error)
278+
const message = (error as Error).message
279+
expect(message).toContain('document library "Policies"')
280+
expect(message).toContain('"HR"')
281+
expect(message).toContain('"Onboarding"')
282+
expect(message).not.toContain('document library "Documents"')
283+
expect(message).not.toContain('Shared Documents" should be omitted')
284+
})
285+
286+
it('still offers the prefix hint when the path names the default library itself', async () => {
287+
mockGraph({
288+
...defaultDriveRoute,
289+
...sitesDrivesRoute,
290+
...rootChildren(DEFAULT_DRIVE_ID, [folder('d1', 'Archive')]),
291+
})
292+
293+
const error = await resolve('Shared Documents/Reports').catch((e: Error) => e)
294+
295+
const message = (error as Error).message
296+
expect(message).toContain('document library "Documents"')
297+
expect(message).toContain('Shared Documents" should be omitted')
298+
})
299+
267300
it('surfaces a failure to open the default library rather than reporting not-found', async () => {
268301
mockGraph({
269302
[`${GRAPH}/sites/${SITE_ID}/drive?$select=id,name,webUrl`]: { status: 403 },

apps/sim/connectors/sharepoint/sharepoint.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -527,14 +527,25 @@ export async function resolveFolderTarget(
527527
return { driveId: defaultDrive.id, driveName: defaultDriveName, folderId: walked.id }
528528
}
529529

530+
/**
531+
* When the first segment named a real library, that library is the one the
532+
* user meant — report against it and its remainder, not against the default
533+
* library and the full path, which would blame the wrong library and advise
534+
* stripping a prefix that was correct.
535+
*/
536+
const reportDrive = libraryMatch
537+
? { id: libraryMatch.id, name: libraryMatch.name || segments[0] }
538+
: { id: defaultDrive.id, name: defaultDriveName }
539+
530540
throw new Error(
531541
await buildFolderNotFoundMessage(
532542
accessToken,
533-
{ id: defaultDrive.id, name: defaultDriveName },
543+
reportDrive,
534544
siteName || siteUrl,
535545
trimmed,
536-
segments,
546+
libraryMatch ? segments.slice(1) : segments,
537547
drives,
548+
reportDrive.id === defaultDrive.id,
538549
retryOptions
539550
)
540551
)
@@ -572,6 +583,11 @@ function matchesDriveName(drive: Drive, segment: string): boolean {
572583
/**
573584
* Builds a diagnostic failure message naming the site, the library searched,
574585
* the path attempted, and the folders that actually exist at that level.
586+
*
587+
* `searchedDefaultLibrary` gates the advice about stripping a leading library
588+
* name: that hint only applies when the path was interpreted against the site's
589+
* default library, and would be actively misleading when the caller supplied a
590+
* library name that matched.
575591
*/
576592
async function buildFolderNotFoundMessage(
577593
accessToken: string,
@@ -580,6 +596,7 @@ async function buildFolderNotFoundMessage(
580596
rawFolderPath: string,
581597
segments: string[],
582598
drives: Drive[],
599+
searchedDefaultLibrary: boolean,
583600
retryOptions?: RetryOptions
584601
): Promise<string> {
585602
const parts = [
@@ -614,9 +631,11 @@ async function buildFolderNotFoundMessage(
614631
}
615632
}
616633

617-
parts.push(
618-
'The folder path is relative to the document library root, so a leading "Documents" or "Shared Documents" should be omitted unless a folder by that name really exists.'
619-
)
634+
if (searchedDefaultLibrary) {
635+
parts.push(
636+
'The folder path is relative to the document library root, so a leading "Documents" or "Shared Documents" should be omitted unless a folder by that name really exists.'
637+
)
638+
}
620639

621640
return parts.join(' ')
622641
}

0 commit comments

Comments
 (0)