Skip to content

Commit 8986fd9

Browse files
Sarath1018claude
andauthored
fix(data-fabric): route expansionLevel to URL query string on queryRecordsById (#531)
* fix(data-fabric): route expansionLevel to URL query string on queryRecordsById DF record POST endpoints honour `expansionLevel` only as a URL query param, not from the request body. The rest of the query options (filters, sort, aggregates, etc.) are still sent in the POST body. Adds a generic `urlParams` hook to PaginationHelpers.getAll that keeps declared keys in the URL query string regardless of HTTP method — body on POST, merged into params on GET — for both the paginated and non-paginated flows. `queryRecordsById` wires `expansionLevel` through it so non-zero levels actually expand reference fields again. Also stops base.ts from folding caller `options.params` into the POST body and nulling out the URL params, so URL query params survive on POST. Adds two integration tests covering all expansion levels (0-3): one for the system reference field `CreatedBy`, and one that builds an isolated source→target schema with a custom RELATIONSHIP field to verify the user-defined column inflates per level. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(data-fabric): skip RELATIONSHIP expansion test under PAT auth entities.create() needs the entity.schema.write scope, which PAT tokens cannot hold (returns insufficient_scope). The CreatedBy expansionLevel test already validates the fix under PAT; keep this body for OAuth runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(pagination): rename urlParams to queryParams queryParams is the recognized HTTP nomenclature and the existing convention in this codebase for query-string params passed via params: (see process-instances.ts, buckets.ts). urlParams elsewhere refers to URLSearchParams over window.location — a different concept. Also re-skip the RELATIONSHIP expansion integration test (needs entity.schema.write, unavailable to PAT) and refine the queryRecordsById comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(data-fabric): use describe.skip for RELATIONSHIP expansion test Match the file's convention for schema-write tests (describe.skip), which need the entity.schema.write scope PAT tokens can't hold. it.skip on a lone test was inconsistent with the surrounding describe.skip blocks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9180f14 commit 8986fd9

8 files changed

Lines changed: 254 additions & 24 deletions

File tree

src/services/base.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,17 +172,16 @@ export class BaseService {
172172
// Prepare request parameters based on pagination type
173173
const requestParams = this.preparePaginationRequestParams(paginationType, params, options.pagination);
174174

175-
// For POST requests, merge pagination params into body and set params to undefined; for GET, use query params
175+
// Route pagination state to wherever the API expects it (body for POST, URL for GET).
176+
// Caller-supplied options.body / options.params are respected as-is — the api-client
177+
// already handles params (URL) and body (request body) independently for every method.
176178
if (method.toUpperCase() === 'POST') {
177179
const existingBody = (options.body && typeof options.body === 'object') ? options.body as Record<string, unknown> : {};
178180
options.body = {
179181
...existingBody,
180-
...options.params,
181182
...requestParams
182183
};
183-
options.params = undefined;
184184
} else {
185-
// Merge pagination parameters with existing parameters
186185
options.params = {
187186
...options.params,
188187
...requestParams

src/services/data-fabric/entities.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -625,15 +625,15 @@ export class EntityService extends BaseService implements EntityServiceModel {
625625
id: string,
626626
options?: T
627627
): Promise<T extends HasPaginationOptions<T> ? PaginatedResponse<EntityRecord> : NonPaginatedResponse<EntityRecord>> {
628-
// folderKey is header-only — destructure it out so PaginationHelpers doesn't include
629-
// it in the POST body alongside the query filters.
630-
const { folderKey, ...rest } = options ?? {};
628+
// folderKey is header-only; expansionLevel must be sent as a query param by PaginationHelpers.
629+
const { folderKey, expansionLevel, ...rest } = options ?? {};
631630
const downstreamOptions = options === undefined ? undefined : (rest as T);
632631
return PaginationHelpers.getAll({
633632
serviceAccess: this.createPaginationServiceAccess(),
634633
getEndpoint: () => DATA_FABRIC_ENDPOINTS.ENTITY.QUERY_BY_ID(id),
635634
method: HTTP_METHODS.POST,
636635
headers: createHeaders({ [FOLDER_KEY]: folderKey }),
636+
queryParams: createParams({ expansionLevel }),
637637
pagination: {
638638
paginationType: PaginationType.OFFSET,
639639
itemsField: ENTITY_PAGINATION.ITEMS_FIELD,
@@ -644,7 +644,7 @@ export class EntityService extends BaseService implements EntityServiceModel {
644644
countParam: ENTITY_OFFSET_PARAMS.COUNT_PARAM
645645
}
646646
},
647-
excludeFromPrefix: ['expansionLevel', 'filterGroup', 'selectedFields', 'sortOptions', 'aggregates', 'groupBy']
647+
excludeFromPrefix: ['filterGroup', 'selectedFields', 'sortOptions', 'aggregates', 'groupBy']
648648
}, downstreamOptions);
649649
}
650650

src/utils/pagination/helpers.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ export class PaginationHelpers {
178178
headers: providedHeaders,
179179
paginationParams,
180180
additionalParams,
181+
queryParams,
181182
transformFn,
182183
method = HTTP_METHODS.GET,
183184
options = {}
@@ -186,13 +187,20 @@ export class PaginationHelpers {
186187
const endpoint = getEndpoint(folderId);
187188
const headers = providedHeaders ?? (folderId ? createHeaders({ [FOLDER_ID]: folderId }) : {});
188189

190+
// On POST, the caller's options go in the body; queryParams stays in the URL.
191+
// On GET, everything is URL — queryParams merges with additionalParams.
192+
const isPost = method === HTTP_METHODS.POST;
193+
const requestSpec = isPost
194+
? { body: additionalParams, params: queryParams }
195+
: { params: { ...additionalParams, ...queryParams } };
196+
189197
const paginatedResponse = await serviceAccess.requestWithPagination<T>(
190198
method,
191199
endpoint,
192200
paginationParams,
193201
{
194202
headers,
195-
params: additionalParams,
203+
...requestSpec,
196204
pagination: {
197205
paginationType: options.paginationType || PaginationType.OFFSET,
198206
itemsField: options.itemsField || DEFAULT_ITEMS_FIELD,
@@ -230,6 +238,7 @@ export class PaginationHelpers {
230238
folderId,
231239
headers: providedHeaders,
232240
additionalParams,
241+
queryParams,
233242
transformFn,
234243
method = HTTP_METHODS.GET,
235244
options = {}
@@ -249,13 +258,13 @@ export class PaginationHelpers {
249258
response = await serviceAccess.post<any>(
250259
endpoint,
251260
additionalParams,
252-
{ headers }
261+
{ headers, params: queryParams }
253262
);
254263
} else {
255264
response = await serviceAccess.get<any>(
256265
endpoint,
257266
{
258-
params: additionalParams,
267+
params: { ...additionalParams, ...queryParams },
259268
headers
260269
}
261270
);
@@ -309,7 +318,7 @@ export class PaginationHelpers {
309318
const excludeKeys = config.excludeFromPrefix || [];
310319
const keysToPrefix = Object.keys(processedOptions).filter(k => !excludeKeys.includes(k));
311320
const prefixedOptions = addPrefixToKeys(processedOptions, ODATA_PREFIX, keysToPrefix);
312-
321+
313322
// Default pagination options
314323
const paginationOptions = {
315324
paginationType: PaginationType.OFFSET,
@@ -327,6 +336,7 @@ export class PaginationHelpers {
327336
headers: config.headers,
328337
paginationParams: cursor ? { cursor, pageSize } : jumpToPage !== undefined ? { jumpToPage, pageSize } : { pageSize },
329338
additionalParams: prefixedOptions,
339+
queryParams: config.queryParams,
330340
transformFn: config.transformFn,
331341
method: config.method,
332342
options: {
@@ -345,6 +355,7 @@ export class PaginationHelpers {
345355
folderId,
346356
headers: config.headers,
347357
additionalParams: prefixedOptions,
358+
queryParams: config.queryParams,
348359
transformFn: config.transformFn,
349360
method: config.method,
350361
options: {

src/utils/pagination/internal-types.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ export interface GetAllPaginatedParams<T, R = T> {
7474
headers?: Record<string, string>;
7575
paginationParams: PaginationOptions;
7676
additionalParams: Record<string, any>;
77+
/** URL query params kept in the URL regardless of HTTP method. */
78+
queryParams?: Record<string, string | number | boolean>;
7779
/**
7880
* Optional function to transform API response items.
7981
*/
@@ -107,6 +109,8 @@ export interface GetAllNonPaginatedParams<T, R = T> {
107109
/** Pre-resolved request headers. Overrides the helper's auto-built folder header from `folderId`. */
108110
headers?: Record<string, string>;
109111
additionalParams: Record<string, any>;
112+
/** URL query params kept in the URL regardless of HTTP method. */
113+
queryParams?: Record<string, string | number | boolean>;
110114
/**
111115
* Optional function to transform API response items.
112116
*/
@@ -236,9 +240,16 @@ export interface GetAllConfig<TRaw, TTransformed = TRaw> {
236240
/** Keys to exclude from ODATA prefix transformation */
237241
excludeFromPrefix?: string[];
238242

243+
/**
244+
* Pre-built URL query parameters that always travel in the URL query string,
245+
* regardless of HTTP method. Use `createParams({ ... })` to build this.
246+
* Typical use: response-shape modifiers on POST endpoints (e.g. `expansionLevel`).
247+
*/
248+
queryParams?: Record<string, string | number | boolean>;
249+
239250
/** HTTP method to use for the request (default: 'GET') */
240251
method?: HttpMethodType;
241252

242253
/** Pre-resolved request headers. Overrides the helper's auto-built folder header from `folderId`. */
243254
headers?: Record<string, string>;
244-
}
255+
}

tests/integration/shared/data-fabric/entities.integration.test.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,135 @@ describe.each(modes)('Data Fabric Entities - Integration Tests [%s]', (mode) =>
829829
expect(typeof row.total).toBe('number');
830830
expect(row.total).toBeGreaterThanOrEqual(0);
831831
});
832+
833+
// Regression guard: DF reads `expansionLevel` only from the URL on POST record endpoints.
834+
// If the SDK sends it in the body, DF silently ignores it and every level collapses to L0.
835+
it('should expand reference fields at each expansionLevel (0-3)', async () => {
836+
const { entities } = getServices();
837+
const config = getTestConfig();
838+
const entityId = config.dataFabricTestEntityId || testEntityId;
839+
if (!entityId) {
840+
throw new Error('No entity ID available for testing');
841+
}
842+
843+
const levels = [0, 1, 2, 3] as const;
844+
const responses = await Promise.all(
845+
levels.map(level => entities.queryRecordsById(entityId, { expansionLevel: level, pageSize: 1 })),
846+
);
847+
848+
responses.forEach((resp, i) => {
849+
expect(resp, `expansionLevel=${levels[i]} returned no response`).toBeDefined();
850+
expect(Array.isArray(resp.items), `expansionLevel=${levels[i]} items not an array`).toBe(true);
851+
});
852+
853+
if (responses.some(r => r.items.length === 0)) {
854+
throw new Error('Test entity has no records — expansionLevel diff cannot be verified. Insert at least one record into DATA_FABRIC_TEST_ENTITY_ID.');
855+
}
856+
857+
const records = responses.map(r => r.items[0] as Record<string, any>);
858+
const [l0Record, l1Record, l2Record, l3Record] = records;
859+
860+
// CreatedBy is a system reference field on every DF record.
861+
// L0: raw GUID string. L1+: object envelope with Id.
862+
expect(typeof l0Record.CreatedBy).toBe('string');
863+
864+
for (const [level, rec] of [[1, l1Record], [2, l2Record], [3, l3Record]] as const) {
865+
expect(typeof rec.CreatedBy, `L${level} CreatedBy should be object`).toBe('object');
866+
expect(rec.CreatedBy, `L${level} CreatedBy should not be null`).not.toBeNull();
867+
expect(rec.CreatedBy, `L${level} CreatedBy should have Id`).toHaveProperty('Id');
868+
}
869+
870+
// L2 inflates the L1 envelope into the referenced user's full record.
871+
expect(Object.keys(l2Record.CreatedBy).length).toBeGreaterThan(
872+
Object.keys(l1Record.CreatedBy).length,
873+
);
874+
875+
// L3 must remain at least as expanded as L2 (deeper nesting is schema-dependent, but never shrinks).
876+
expect(Object.keys(l3Record.CreatedBy).length).toBeGreaterThanOrEqual(
877+
Object.keys(l2Record.CreatedBy).length,
878+
);
879+
});
880+
881+
// Custom user-defined RELATIONSHIP fields must follow the same expansion rules as
882+
// system reference fields, but verifying this requires creating an isolated
883+
// source→target schema at runtime. entities.create() needs the entity.schema.write
884+
// scope, which PAT tokens cannot hold (insufficient_scope) — so this stays skipped,
885+
// matching the other schema-write describe.skip blocks below. The CreatedBy test
886+
// above already validates the expansionLevel-via-URL fix under PAT.
887+
describe.skip('RELATIONSHIP field expansion (requires schema-write scope)', () => {
888+
it('should expand a custom RELATIONSHIP field at each expansionLevel (0-3)', async () => {
889+
const { entities } = getServices();
890+
const stamp = generateRandomString(8).toLowerCase();
891+
892+
// 1. Target entity with a user-defined string field we can assert on at L2+.
893+
const targetId = await entities.create(`sdk_target_${stamp}`, [
894+
{ fieldName: 'label', type: EntityFieldDataType.STRING, isRequired: true },
895+
]);
896+
createdEntityIds.push(targetId);
897+
898+
const targetMeta = await entities.getById(targetId);
899+
const targetPk = targetMeta.fields.find(f => f.isPrimaryKey);
900+
if (!targetPk?.id) {
901+
throw new Error(`Target entity ${targetId} has no primary-key field`);
902+
}
903+
904+
// 2. Source entity with a RELATIONSHIP field bound to target.Id.
905+
const sourceId = await entities.create(`sdk_source_${stamp}`, [
906+
{ fieldName: 'name', type: EntityFieldDataType.STRING },
907+
{
908+
fieldName: 'parent',
909+
type: EntityFieldDataType.RELATIONSHIP,
910+
referenceEntityId: targetId,
911+
referenceFieldId: targetPk.id,
912+
},
913+
]);
914+
createdEntityIds.push(sourceId);
915+
916+
// 3. Insert a target record so the FK has something to resolve.
917+
const labelValue = `Target_${stamp}`;
918+
const targetInsert = await entities.insertRecordById(targetId, { label: labelValue });
919+
const targetRecordId = targetInsert.Id;
920+
registerResource('entityRecords', { entityId: targetId, recordIds: [targetRecordId] });
921+
922+
// 4. Insert the source record with the FK populated.
923+
const sourceInsert = await entities.insertRecordById(sourceId, {
924+
name: `Source_${stamp}`,
925+
parent: targetRecordId,
926+
});
927+
registerResource('entityRecords', { entityId: sourceId, recordIds: [sourceInsert.Id] });
928+
929+
// 5. Query the source at every expansion level.
930+
const levels = [0, 1, 2, 3] as const;
931+
const responses = await Promise.all(
932+
levels.map(level => entities.queryRecordsById(sourceId, { expansionLevel: level, pageSize: 10 })),
933+
);
934+
935+
const sourceRecords = responses.map(r =>
936+
(r.items as Record<string, any>[]).find(item => item.Id === sourceInsert.Id),
937+
);
938+
sourceRecords.forEach((rec, i) => {
939+
expect(rec, `expansionLevel=${levels[i]} did not return the inserted source record`).toBeDefined();
940+
});
941+
const [l0, l1, l2, l3] = sourceRecords as Record<string, any>[];
942+
943+
// L0: FK is the raw target record Id string.
944+
expect(typeof l0.parent).toBe('string');
945+
expect(l0.parent).toBe(targetRecordId);
946+
947+
// L1+: FK inflates into an object envelope carrying the target Id.
948+
for (const [level, rec] of [[1, l1], [2, l2], [3, l3]] as const) {
949+
expect(typeof rec.parent, `L${level} parent should be object`).toBe('object');
950+
expect(rec.parent, `L${level} parent should not be null`).not.toBeNull();
951+
expect(rec.parent, `L${level} parent should carry target Id`).toHaveProperty('Id', targetRecordId);
952+
}
953+
954+
// L2 surfaces the user-defined `label` field from the target record.
955+
expect(l2.parent.label).toBe(labelValue);
956+
957+
// L3 cannot shrink relative to L2.
958+
expect(Object.keys(l3.parent).length).toBeGreaterThanOrEqual(Object.keys(l2.parent).length);
959+
});
960+
});
832961
});
833962

834963
describe('importRecordsById', () => {

tests/unit/services/base.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ describe('BaseService Unit Tests', () => {
407407
expect(params).not.toHaveProperty(ODATA_OFFSET_PARAMS.COUNT_PARAM);
408408
});
409409

410-
it('should merge pagination params into the body for POST and clear query params', async () => {
410+
it('should merge pagination params into body for POST while leaving caller params in the URL', async () => {
411411
mockApiClient.post.mockResolvedValue({ value: [TEST_RESPONSE], totalRecordCount: 1 });
412412

413413
await service.exposedRequestWithPagination(
@@ -423,14 +423,14 @@ describe('BaseService Unit Tests', () => {
423423

424424
const [calledPath, calledBody, calledOptions] = mockApiClient.post.mock.calls[0];
425425
expect(calledPath).toBe(TEST_PATH);
426+
// Pagination state lands in body. Caller's body keys are preserved. No URL keys leak in.
426427
expect(calledBody).toEqual({
427428
filter: 'abc',
428-
extra: 'query',
429429
[ODATA_OFFSET_PARAMS.PAGE_SIZE_PARAM]: 10,
430430
[ODATA_OFFSET_PARAMS.COUNT_PARAM]: true,
431431
});
432-
expect(calledOptions.params).toBeUndefined();
433-
expect(calledOptions.body).toEqual(calledBody);
432+
// Caller's URL params are respected — no longer clobbered.
433+
expect(calledOptions.params).toEqual({ extra: 'query' });
434434
});
435435

436436
it('should fall back to itemsCount heuristic for hasNextPage when totalCount is missing', async () => {

tests/unit/services/data-fabric/entities.test.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1505,7 +1505,7 @@ describe("EntityService Unit Tests", () => {
15051505
itemsField: "value",
15061506
totalCountField: "totalRecordCount",
15071507
}),
1508-
excludeFromPrefix: expect.arrayContaining(["expansionLevel", "filterGroup", "sortOptions"]),
1508+
excludeFromPrefix: expect.arrayContaining(["filterGroup", "sortOptions"]),
15091509
}),
15101510
undefined,
15111511
);
@@ -1557,24 +1557,28 @@ describe("EntityService Unit Tests", () => {
15571557
);
15581558
});
15591559

1560-
it("should pass expansionLevel through excludeFromPrefix without ODATA prefix", async () => {
1560+
it("should route expansionLevel to queryParams and strip it from the body options", async () => {
15611561
let capturedConfig: any;
1562-
vi.mocked(PaginationHelpers.getAll).mockImplementation(async (config) => {
1562+
let capturedDownstream: any;
1563+
vi.mocked(PaginationHelpers.getAll).mockImplementation(async (config, downstream) => {
15631564
capturedConfig = config;
1565+
capturedDownstream = downstream;
15641566
return { items: [], totalCount: 0 };
15651567
});
15661568

15671569
await entityService.queryRecordsById(ENTITY_TEST_CONSTANTS.ENTITY_ID, {
15681570
expansionLevel: ENTITY_TEST_CONSTANTS.EXPANSION_LEVEL,
15691571
});
15701572

1571-
// expansionLevel is in excludeFromPrefix — no URL manipulation
15721573
expect(capturedConfig.getEndpoint()).toBe(
15731574
DATA_FABRIC_ENDPOINTS.ENTITY.QUERY_BY_ID(ENTITY_TEST_CONSTANTS.ENTITY_ID),
15741575
);
1575-
expect(capturedConfig.getEndpoint()).not.toContain("expansionLevel");
1576-
expect(capturedConfig.excludeFromPrefix).toContain("expansionLevel");
1577-
// no processParametersFn — same pattern as getAllRecords
1576+
// expansionLevel is hoisted into queryParams (URL) — same pattern as insertRecordById / updateRecordById.
1577+
expect(capturedConfig.queryParams).toEqual({
1578+
expansionLevel: ENTITY_TEST_CONSTANTS.EXPANSION_LEVEL,
1579+
});
1580+
// It must not also appear in the downstream (body) options.
1581+
expect(capturedDownstream).not.toHaveProperty("expansionLevel");
15781582
expect(capturedConfig.processParametersFn).toBeUndefined();
15791583
});
15801584

0 commit comments

Comments
 (0)