Skip to content

Commit 507663f

Browse files
BillLeoutsakosvl346Bill Leoutsakos
andauthored
fix(selectors): paginate SharePoint sites and lists (#7335)
Co-authored-by: Bill Leoutsakos <billleoutsakos@Bills-MacBook-Pro.local>
1 parent 2598f44 commit 507663f

3 files changed

Lines changed: 142 additions & 43 deletions

File tree

apps/sim/lib/selectors/manifest.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ export const selectorManifest = {
189189
'pipedrive.pipelines': providerSelector([], { detail: true }),
190190
'sharepoint.lists': providerSelector(['siteId'], {
191191
readiness: { all: ['oauthCredential', 'siteId'] },
192+
listMode: 'paginated',
192193
detail: true,
193194
}),
194195
'trello.boards': providerSelector([], { detail: true }),
@@ -254,7 +255,11 @@ export const selectorManifest = {
254255
}),
255256
'onedrive.files': providerSelector(['mimeType'], { listMode: 'paginated', detail: true }),
256257
'onedrive.folders': providerSelector(['driveId'], { listMode: 'paginated', detail: true }),
257-
'sharepoint.sites': providerSelector([], { detail: true }),
258+
'sharepoint.sites': providerSelector([], {
259+
listMode: 'paginated',
260+
search: true,
261+
detail: true,
262+
}),
258263
'microsoft.excel': providerSelector(['driveId'], {
259264
listMode: 'paginated',
260265
search: true,

apps/sim/lib/selectors/server/providers/sharepoint.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,21 @@ function detailArgs(
3737
}
3838
}
3939

40+
function listArgs(
41+
selectorKey: 'sharepoint.lists' | 'sharepoint.sites',
42+
cursor?: string,
43+
search?: string
44+
): ExecuteServerSelectorArgs {
45+
return {
46+
...detailArgs(selectorKey, ''),
47+
request: {
48+
kind: 'list',
49+
...(cursor ? { cursor } : {}),
50+
...(search ? { search } : {}),
51+
},
52+
}
53+
}
54+
4055
describe('SharePoint server selector adapter', () => {
4156
beforeEach(() => {
4257
vi.clearAllMocks()
@@ -46,6 +61,76 @@ describe('SharePoint server selector adapter', () => {
4661

4762
afterAll(() => vi.unstubAllGlobals())
4863

64+
it.each([
65+
{
66+
selectorKey: 'sharepoint.sites' as const,
67+
search: ' Engineering ',
68+
firstValue: { id: 'site-1', name: 'Engineering' },
69+
firstItem: { id: 'site-1', label: 'Engineering' },
70+
secondValue: { id: 'site-2', name: 'Operations' },
71+
secondItem: { id: 'site-2', label: 'Operations' },
72+
nextCursor: 'https://graph.microsoft.com/v1.0/sites?search=Engineering&$skiptoken=next',
73+
},
74+
{
75+
selectorKey: 'sharepoint.lists' as const,
76+
search: undefined,
77+
firstValue: { id: 'list-1', displayName: 'Planning', list: { hidden: false } },
78+
firstItem: { id: 'list-1', label: 'Planning' },
79+
secondValue: { id: 'list-2', displayName: 'Operations', list: { hidden: false } },
80+
secondItem: { id: 'list-2', label: 'Operations' },
81+
nextCursor:
82+
'https://graph.microsoft.com/v1.0/sites/contoso.sharepoint.com%2Csite%2Cweb/lists?$skiptoken=next',
83+
},
84+
])('paginates $selectorKey only when its cursor is requested', async (testCase) => {
85+
mockFetch
86+
.mockResolvedValueOnce(
87+
new Response(
88+
JSON.stringify({ value: [testCase.firstValue], '@odata.nextLink': testCase.nextCursor }),
89+
{ status: 200 }
90+
)
91+
)
92+
.mockResolvedValueOnce(
93+
new Response(JSON.stringify({ value: [testCase.secondValue] }), { status: 200 })
94+
)
95+
96+
const first = await sharepointSelectorAttachments[testCase.selectorKey].execute(
97+
listArgs(testCase.selectorKey, undefined, testCase.search)
98+
)
99+
100+
expect(first).toEqual({
101+
kind: 'list',
102+
items: [testCase.firstItem],
103+
nextCursor: testCase.nextCursor,
104+
})
105+
expect(mockFetch).toHaveBeenCalledTimes(1)
106+
if (testCase.search) {
107+
expect(new URL(String(mockFetch.mock.calls[0]?.[0])).searchParams.get('search')).toBe(
108+
'Engineering'
109+
)
110+
}
111+
112+
const second = await sharepointSelectorAttachments[testCase.selectorKey].execute(
113+
listArgs(testCase.selectorKey, testCase.nextCursor, testCase.search)
114+
)
115+
116+
expect(second).toEqual({ kind: 'list', items: [testCase.secondItem] })
117+
expect(String(mockFetch.mock.calls[1]?.[0])).toBe(testCase.nextCursor)
118+
expect(mockFetch).toHaveBeenCalledTimes(2)
119+
})
120+
121+
it('rejects a Graph cursor for another SharePoint resource', async () => {
122+
await expect(
123+
sharepointSelectorAttachments['sharepoint.lists'].execute(
124+
listArgs(
125+
'sharepoint.lists',
126+
'https://graph.microsoft.com/v1.0/sites/another-site/lists?$skiptoken=next'
127+
)
128+
)
129+
).rejects.toMatchObject({ name: 'SelectorContextUnavailableError' })
130+
expect(mockResolveSelectorOAuthAccessToken).not.toHaveBeenCalled()
131+
expect(mockFetch).not.toHaveBeenCalled()
132+
})
133+
49134
it('hydrates a selected list directly within its site', async () => {
50135
mockFetch.mockResolvedValueOnce(
51136
new Response(JSON.stringify({ id: 'list-1', displayName: 'Planning' }), { status: 200 })

apps/sim/lib/selectors/server/providers/sharepoint.ts

Lines changed: 51 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,19 @@ import {
99
SelectorContextUnavailableError,
1010
SelectorOptionsUnavailableError,
1111
} from '@/lib/selectors/server/errors'
12-
import { flatSelectorResult } from '@/lib/selectors/server/providers/flat-results'
1312
import { fetchProviderJson } from '@/lib/selectors/server/providers/provider-http'
1413
import {
1514
detailSelectorResult,
1615
type ExecuteServerSelectorArgs,
16+
listSelectorResult,
17+
requireListRequest,
1718
type ServerSelectorAttachmentMap,
1819
} from '@/lib/selectors/server/types'
1920
import type { SafeSelectorOption } from '@/lib/selectors/types'
2021
import { assertGraphNextPageUrl, getGraphNextPageUrl } from '@/tools/sharepoint/utils'
2122

2223
type SharePointSelectorKey = Extract<ServerSelectorKey, 'sharepoint.lists' | 'sharepoint.sites'>
2324

24-
const MAX_GRAPH_PAGES = 10
25-
2625
const sharepointCredential = {
2726
kind: 'stored',
2827
field: 'oauthCredential',
@@ -44,24 +43,43 @@ async function graphToken(args: ExecuteServerSelectorArgs): Promise<string> {
4443
})
4544
}
4645

47-
async function drainGraph<T>(
46+
interface GraphPage<T> {
47+
items: T[]
48+
nextCursor?: string
49+
}
50+
51+
function graphPageUrl(cursor: string | undefined, initialUrl: string): string {
52+
if (!cursor) return initialUrl
53+
let cursorUrl: string
54+
try {
55+
cursorUrl = assertGraphNextPageUrl(cursor)
56+
} catch {
57+
throw new SelectorContextUnavailableError()
58+
}
59+
if (new URL(cursorUrl).pathname !== new URL(initialUrl).pathname) {
60+
throw new SelectorContextUnavailableError()
61+
}
62+
return cursorUrl
63+
}
64+
65+
async function fetchGraphPage<T>(
4866
args: ExecuteServerSelectorArgs,
4967
initialUrl: string
50-
): Promise<{ values: T[]; truncated: boolean }> {
68+
): Promise<GraphPage<T>> {
69+
const request = requireListRequest(args.selectorKey, args.request)
70+
const requestUrl = graphPageUrl(request.cursor, initialUrl)
5171
const token = await graphToken(args)
52-
const values: T[] = []
53-
let nextUrl: string | undefined = initialUrl
54-
for (let page = 0; page < MAX_GRAPH_PAGES && nextUrl; page++) {
55-
const data = await fetchProviderJson<{ value?: T[] } & Record<string, unknown>>(nextUrl, {
56-
headers: { Authorization: `Bearer ${token}` },
57-
signal: args.signal,
58-
redirect: 'error',
59-
})
60-
if (Array.isArray(data.value)) values.push(...data.value)
61-
const nextLink = getGraphNextPageUrl(data)
62-
nextUrl = nextLink ? assertGraphNextPageUrl(nextLink) : undefined
72+
const data = await fetchProviderJson<{ value?: T[] } & Record<string, unknown>>(requestUrl, {
73+
headers: { Authorization: `Bearer ${token}` },
74+
signal: args.signal,
75+
redirect: 'error',
76+
})
77+
const nextLink = getGraphNextPageUrl(data)
78+
const nextCursor = nextLink ? graphPageUrl(nextLink, initialUrl) : undefined
79+
return {
80+
items: Array.isArray(data.value) ? data.value : [],
81+
...(nextCursor ? { nextCursor } : {}),
6382
}
64-
return { values, truncated: Boolean(nextUrl) }
6583
}
6684

6785
function requireSiteId(value: string | undefined): string {
@@ -130,7 +148,7 @@ async function getSite(
130148

131149
async function listLists(args: ExecuteServerSelectorArgs) {
132150
const siteId = requireSiteId(args.context.siteId)
133-
const result = await drainGraph<{
151+
const page = await fetchGraphPage<{
134152
id: string
135153
displayName: string
136154
list?: { hidden?: boolean }
@@ -139,21 +157,26 @@ async function listLists(args: ExecuteServerSelectorArgs) {
139157
`https://graph.microsoft.com/v1.0/sites/${encodeURIComponent(siteId)}/lists?$select=id,displayName,description,webUrl,list&$top=999`
140158
)
141159
return {
142-
items: result.values
160+
items: page.items
143161
.filter((list) => list.list?.hidden !== true)
144162
.map((list) => ({ id: list.id, label: list.displayName })),
145-
truncated: result.truncated,
163+
nextCursor: page.nextCursor,
146164
}
147165
}
148166

149167
async function listSites(args: ExecuteServerSelectorArgs) {
150-
const result = await drainGraph<{ id: string; name: string; displayName?: string }>(
168+
const request = requireListRequest(args.selectorKey, args.request)
169+
const url = new URL('https://graph.microsoft.com/v1.0/sites')
170+
url.searchParams.set('search', request.search?.trim() || '*')
171+
url.searchParams.set('$select', 'id,name,displayName,webUrl,createdDateTime,lastModifiedDateTime')
172+
url.searchParams.set('$top', '999')
173+
const page = await fetchGraphPage<{ id: string; name: string; displayName?: string }>(
151174
args,
152-
'https://graph.microsoft.com/v1.0/sites?search=*&$select=id,name,displayName,webUrl,createdDateTime,lastModifiedDateTime&$top=999'
175+
url.toString()
153176
)
154177
return {
155-
items: result.values.map((site) => ({ id: site.id, label: site.displayName || site.name })),
156-
truncated: result.truncated,
178+
items: page.items.map((site) => ({ id: site.id, label: site.displayName || site.name })),
179+
nextCursor: page.nextCursor,
157180
}
158181
}
159182

@@ -165,15 +188,8 @@ export const sharepointSelectorAttachments = {
165188
if (args.request.kind === 'detail') {
166189
return detailSelectorResult(await getList(args, args.request.id))
167190
}
168-
const result = await listLists(args)
169-
return flatSelectorResult(
170-
args.request,
171-
result.items,
172-
false,
173-
result.truncated
174-
? { truncated: { reason: 'provider-cap', pages: MAX_GRAPH_PAGES } }
175-
: undefined
176-
)
191+
const page = await listLists(args)
192+
return listSelectorResult(page.items, page.nextCursor)
177193
},
178194
},
179195
'sharepoint.sites': {
@@ -183,15 +199,8 @@ export const sharepointSelectorAttachments = {
183199
if (args.request.kind === 'detail') {
184200
return detailSelectorResult(await getSite(args, args.request.id))
185201
}
186-
const result = await listSites(args)
187-
return flatSelectorResult(
188-
args.request,
189-
result.items,
190-
false,
191-
result.truncated
192-
? { truncated: { reason: 'provider-cap', pages: MAX_GRAPH_PAGES } }
193-
: undefined
194-
)
202+
const page = await listSites(args)
203+
return listSelectorResult(page.items, page.nextCursor)
195204
},
196205
},
197206
} satisfies ServerSelectorAttachmentMap<SharePointSelectorKey>

0 commit comments

Comments
 (0)