fixes three related pagination bugs in the command's org user fetch logic.#233
Open
cs-raj wants to merge 1 commit into
Open
fixes three related pagination bugs in the command's org user fetch logic.#233cs-raj wants to merge 1 commit into
cs-raj wants to merge 1 commit into
Conversation
🔒 Security Scan Results
⏱️ SLA Breach Summary
🟡 Medium Severity - SLA Breached Issues (with fixes)Showing 4 issue(s) that have exceeded the 90-day SLA threshold:
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
❌ BUILD FAILED - Security checks failed Please review and fix the security vulnerabilities before merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Port of fix/DX-3943 to v1-dev. Fixes three related pagination bugs in the
export-to-csvcommand's org user fetch logic.Problem
Bug 1 — Org owners only got the first page of users
getOrgUsershad a special branch foris_owner === truethat calledgetInvitations()with no parameters and resolved immediately. Ownerswith more than
config.limitusers silently received a truncated list.Bug 2 — Wrong access-denied guard
The guard for non-owners checked
!organization.getInvitations(whethera method existed on the object) instead of
!organization.is_owner.This was always falsy and the check was effectively dead.
Bug 3 — Last page items dropped in pagination loop
getUsersstopped paginating whenusers.items.length === 0. But theactual final page (a partial page with fewer items than the limit) was
fetched, its items ignored, and then an extra empty-page round-trip was
made before stopping.
Changes
src/utils/api-client.tsis_owner === trueearly-exit branch; owners now gothrough the same paginated
getUserspath as admins.!organization.getInvitations→!organization.is_owner.getUsersfrom!users.items.length→users.items.length < params.limit, and correctly appends thepartial last page's items before returning.
limit: 100→limit: config.limit.test/unit/utils/api-client.test.tsgetOrgUserstest suite with three cases: pagination forowners, pagination for admins, and access-denied rejection.
Test plan
getOrgUsersreturns all pages for org owners (not just page 1)getOrgUsersreturns all pages for org adminsgetOrgUsersrejects withERROR_ADMIN_ACCESS_DENIEDfor users with neither rolenpm run test:unit— all api-client tests pass