Skip to content

Commit 38b20aa

Browse files
akshay-vizCopilot
andcommitted
fix(model-apps): paginate the role-privilege read instead of capping at 5000
Found by a LIVE run against a real environment, which is the only place it could have shown up: with `top: 5000`, reading a System Administrator role returned EXACTLY 5000 rows. Paginated, the same role returns 7119 -- so 2119 privileges, 30% of the role, were being silently dropped. A truncated page is the worst possible shape for this check. `compareRolePrivileges` asks whether the role holds each DECLARED privilege; a privilege that merely fell off the end of page one is indistinguishable from one the role does not hold, so verify reports a correctly configured role as MISSING privileges. Same cry-wolf class as the two earlier fixes in this PR, but this one is silent: nothing in the response says the list was cut short. Follows @odata.nextLink to completion instead, matching what fetchAppsForPages already does for the same reason. Deliberately NOT combined with `top`: Dataverse honors $top as a hard cap and omits @odata.nextLink when it is present, and the SDK rejects paginate+top for exactly that reason -- so asserting the ABSENCE of top matters as much as asserting pagination. Cost is not a concern: the paginated read of 7119 rows completed in 692ms. Red-green verified; restoring `top: 5000` fails the new test. CI could not have caught this -- every unit test stubs queryRecords and returns a short list, so the cap was invisible until a real role exceeded it. The test now pins the query OPTIONS rather than the result, which is the part a stub cannot fake. 1486 tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42626da2-b66f-4162-acaa-b1127ef23d89
1 parent 955b2b9 commit 38b20aa

2 files changed

Lines changed: 28 additions & 1 deletion

File tree

plugins/model-apps/scripts/tests/verify-model-app.test.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,3 +482,23 @@ test('entityPrivileges is ABSENT (not broken) when the client or org url is miss
482482
assert.strictEqual(typeof readerFor(stubSdk(), 'app', { httpClient: { get: async () => ({}) } }).entityPrivileges, 'undefined');
483483
assert.strictEqual(typeof readerFor(stubSdk(), 'app', { envUrl: 'https://contoso.crm.dynamics.com' }).entityPrivileges, 'undefined');
484484
});
485+
486+
test('rolePrivileges paginates and never caps with top', async () => {
487+
// Found by a LIVE run: with the previous op: 5000 a System Administrator role returned
488+
// EXACTLY 5000 rows -- silently truncated. Paginated it returns 7119, so 2119 privileges were
489+
// being dropped. A truncated page is the worst shape for this check: a declared privilege that
490+
// fell off the end reads as NOT HELD, so verify reports a correctly configured role as missing.
491+
// Dataverse honors as a hard cap and omits @odata.nextLink, and the SDK rejects
492+
// paginate+top, so asserting the ABSENCE of top matters as much as the presence of paginate.
493+
const calls = [];
494+
const sdk = stubSdk();
495+
sdk.queryRecords = async (set, options) => { calls.push({ set, options }); return []; };
496+
const read = readerFor(sdk, 'app', { httpClient: { get: async () => ({ status: 200, body: {} }) }, envUrl: 'https://contoso.crm.dynamics.com' });
497+
498+
await read.rolePrivileges('00000000-0000-0000-0000-000000000001');
499+
500+
const q = calls.find((c) => c.set === 'roleprivileges');
501+
assert.ok(q, 'expected a roleprivileges query');
502+
assert.strictEqual(q.options.paginate, true, 'must follow @odata.nextLink to completion');
503+
assert.strictEqual('top' in q.options, false, 'must NOT cap with top -- Dataverse treats it as a hard cap and drops nextLink');
504+
});

plugins/model-apps/scripts/verify-model-app.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,14 @@ function readerFor(sdk, appUnique, opts) {
106106
const rows = await sdk.queryRecords('roleprivileges', {
107107
select: ['privilegeid', 'privilegedepthmask'],
108108
filter: `roleid eq ${roleId}`,
109-
top: 5000,
109+
// Follow @odata.nextLink to completion rather than capping with `top`. Verified live: a
110+
// System Administrator role returned EXACTLY 5000 rows against the previous `top: 5000`,
111+
// i.e. it was silently truncated at the cap. A truncated page is the worst shape for this
112+
// check — a declared privilege that simply fell off the end reads as NOT HELD, so verify
113+
// reports a correctly configured role as missing privileges. Not combined with `top`:
114+
// Dataverse honors $top as a hard cap and omits @odata.nextLink, and the SDK rejects
115+
// paginate+top for exactly that reason.
116+
paginate: true,
110117
});
111118
return (rows || []).map((r) => ({
112119
privilegeId: String((r && r.privilegeid) || ''),

0 commit comments

Comments
 (0)