Update changelog for version 2.9.0: add automatic pagination and rate limit handling - #103
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the NinjaOne PowerShell module to version 2.9.0 with automatic GET pagination and HTML rate-limit retry handling.
Changes:
- Added cursor/activity pagination with a
-PageSizeopt-out. - Added exponential backoff for HTML rate-limit responses.
- Updated module metadata, initialization settings, documentation, and changelog.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Summary | Findings |
|---|---|---|
Source/Public/PSModule/Invoke/Invoke-NinjaOneRequest.ps1 |
Adds rate-limit detection and retries. | Critical: retries may replay mutating requests. Tests are also needed. |
Source/Private/New-NinjaOneGETRequest.ps1 |
Implements pagination and result aggregation. | Moderate: custom-fields pagination must use cursorName. Pagination paths need tests. |
Source/NinjaOne.psd1 |
Updates module version to 2.9.0. | No final review comments. |
Source/Initialisation.ps1 |
Defines retry configuration defaults. | No final review comments. |
docs/NinjaOne/index.mdx |
Documents pagination behavior. | Nit: PageSize ranges are endpoint-specific. |
CHANGELOG.md |
Records the 2.9.0 changes. | No final review comments. |
Suppressed comments (3)
Source/Private/New-NinjaOneGETRequest.ps1:63
- The pagination cursor is written directly into the caller's
NameValueCollection. Public commands such asGet-NinjaOneActivitiesbuild that collection once inbeginbut can invoke this helper repeatedly for pipeline input; after the first device, the next invocation inherits the previous device'solderThanvalue and can skip activities. Clone the query collection before adding pagination state so each invocation leaves its caller-owned filters unchanged.
if ($OlderThanCursor) {
$QueryStringCollection.Set('olderThan', [String]$OlderThanCursor)
}
Source/Private/New-NinjaOneGETRequest.ps1:132
- This assumes every
resultsresponse exposes its next token ascursor.name. The repository's/v2/custom-fieldscontract uses theSupportPaginationResponseshape (ninjaOne-API-core-resources.yaml:11264) and the command passescursorName; that response has nocursor.name, so$NextCursoris null andGet-NinjaOneCustomFieldsSchemastops after the first page. Use the endpoint's actual pagination fields and parameter name, or explicitly exclude this shape from this branch.
$NextCursor = $Result.cursor.name
Source/Private/New-NinjaOneGETRequest.ps1:153
- For
/v2/device/{id}/activities, the repository schema returnsNodeActivitiesResponse, which includeslastNodeActivityId(ninjaOne-API-core-resources.yaml:15439). Reconstructing the aggregate with onlylastActivityIdandactivitiesdrops that device-specific metadata from everyGet-NinjaOneActivities -deviceIdresult. PreservelastNodeActivityIdwhile aggregating, capturing it from the appropriate page.
return [PSCustomObject]@{
lastActivityId = $AccountLastActivityId
activities = $PageResults.ToArray()
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #103 +/- ##
==========================================
+ Coverage 71.43% 73.08% +1.64%
==========================================
Files 317 317
Lines 4292 4436 +144
==========================================
+ Hits 3066 3242 +176
+ Misses 1226 1194 -32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
…for version 2.9.0
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate review findings remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (6)
Source/Private/New-NinjaOneGETRequest.ps1:131
- The classifier treats any response without
activities,results, orresultas raw. API endpoints such as/v2/organizationsand/v2/devicesreturn a bare array but exposepageSizeplusafteras the cursor for the next page, so these calls still stop after the first page instead of providing the advertised automatic pagination. Add anafter-based aggregation path or narrow the release/documentation scope.
} elseif ($Properties -contains 'result') {
'result'
} else {
'raw'
}
Source/Private/New-NinjaOneGETRequest.ps1:64
- The pagination state is only applied inside this
ifblock. When the helper is called without a query collection, it cannot add the returned cursor/olderThanto the next URI, and the pagination checks later in the loop stop after the first page. Pagination should not depend on callers supplying even an empty query collection; initialize a local collection and always build the next-page query from it.
if ($QueryStringCollection) {
$RequestQueryStringCollection = [System.Collections.Specialized.NameValueCollection]::new()
Source/Public/PSModule/Invoke/Invoke-NinjaOneRequest.ps1:95
- For a non-GET HTML response, the retry condition is skipped and execution continues into the JSON conversion path, so callers receive a generic ConvertFrom-Json failure rather than the HTML/rate-limit response described by the comment. Handle this response explicitly for non-GET methods, for example by throwing a rate-limit-specific error while preserving the body, before attempting JSON parsing.
if ($method -eq 'GET' -and $IsRateLimitedResponse) {
if ($Attempt -gt $Script:NRAPIRateLimitMaxRetries) {
throw ('NinjaOne API rate limit exceeded - received an HTML response after {0} attempts.' -f $Attempt)
}
$DelaySeconds = $Script:NRAPIRateLimitInitialDelaySeconds * [Math]::Pow(2, $Attempt - 1)
Tests/NinjaOne.Private.Tests.ps1:1027
- This mock also ignores the request URI, so a regression that uses the account-wide
lastActivityId(999) instead of the last returned activity id (19) still produces two calls and passes all assertions. Assert that the second request containsolderThan=19to cover the behavior described by the test and changelog.
Mock -CommandName Invoke-NinjaOneRequest -ModuleName $ModuleName -MockWith {
$script:CallCount++
if ($script:CallCount -eq 1) {
[pscustomobject]@{
lastActivityId = 999
activities = @([pscustomobject]@{ id = 20 }, [pscustomobject]@{ id = 19 })
Tests/NinjaOne.Private.Tests.ps1:1100
- This test also supplies -pageSize, which disables auto-pagination, so it verifies only that the initial request receives cursorName; it never proves that a returned cursor is sent back using the custom parameter on page 2. Exercise a multi-page call without pageSize and assert the second request contains cursorName=.
Mock -CommandName New-NinjaOneGETRequest -ModuleName $ModuleName -MockWith {
[pscustomobject]@{ results = @(); cursor = [pscustomobject]@{ name = 'next-page' } }
}
$null = Get-NinjaOneCustomFieldsSchema -cursorName 'next-page' -pageSize 25
docs/NinjaOne/index.mdx:22
- The GET helper only auto-pages
results/cursorandactivitiesresponse shapes. Raw-array paginated endpoints such asGet-NinjaOneOrganisationsstill usepageSize/afterand are not auto-paged, so the warning's claim about all GET endpoints and a universal-PageSizeopt-out is misleading. Narrow the wording to the supported shapes or implement the other pagination contracts.
The 2.9.0 release of the NinjaOne PowerShell module introduces automatic pagination for all GET endpoints that support it. If you want to opt out of this behaviour for a specific call, supply `-PageSize` for that endpoint; the module will return only the requested page. The valid page-size range is endpoint-specific, so check the individual commandlet help for the supported range on that API call.
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Lite
…d add cursor preservation in paging activities
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Restrict after pagination to endpoints that explicitly support it.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Source/Private/New-NinjaOneGETRequest.ps1:161
- The checked-in OpenAPI contract for
/v2/custom-fieldspoints toSupportPaginationResponse, which containspage,pageSize,count, andresults, but nocursorproperty. With that documented response,$Result.cursor.nameis null andGet-NinjaOneCustomFieldsSchemastops after the first page even though this change opts it intocursorNamepagination. Handle the endpoint's actual pagination response contract before relying oncursor.name.
$NextCursor = $Result.cursor.name
Source/Public/PSModule/Invoke/Invoke-NinjaOneRequest.ps1:91
ValidateSetacceptsgetas well asGETbut does not canonicalize the value. When a caller passes-Method get, both GET checks in this loop are false, so an HTML rate-limit response is treated as a non-GET response and throws immediately instead of retrying. Use a case-insensitive comparison (or normalize$method) in both conditions.
if ($method -eq 'GET' -and $IsRateLimitedResponse) {
docs/NinjaOne/index.mdx:22
- Use the PowerShell term
cmdlethere;commandletis a misspelling in the new documentation.
The 2.9.0 release of the NinjaOne PowerShell module introduces automatic pagination for all GET endpoints that support it. If you want to opt out of this behaviour for a specific call, supply `-PageSize` for that endpoint; the module will return only the requested page. The valid page-size range is endpoint-specific, so check the individual commandlet help for the supported range on that API call.
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
…aOneRequest; add test for non-paginated responses
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Fix the single-item-page pagination issue before approval.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
Source/Private/New-NinjaOneGETRequest.ps1:134
- Top-level JSON arrays are unrolled by
Invoke-NinjaOneRequestwhenreturn $Resultsemits a single-item array, so$Result -is [Array]is false here. A supportedafterendpoint whose page contains one item is therefore classified asrawand the next page is never requested, bypassing auto-pagination; preserve the array shape or detect it before pipeline enumeration, and cover this case with a single-item-page test.
} elseif ($Result -is [Array]) {
'array'
Source/Private/New-NinjaOneGETRequest.ps1:176
- This direct-array branch only advances with
afterand always reads the last item'sid, so other paginated array contracts are silently truncated. The API spec defines/v2/ticketing/app-user-contactwithpageSize+anchorNaturalId(and items exposenaturalId) and ticket log entries withpageSize+anchorId; neither resource is handled here, so calls without-pageSizereturn only the first page despite the PR's automatic-pagination behavior. Add endpoint-specific anchor strategies (and tests), or explicitly narrow the documented scope.
$NextAfter = ($Page | Select-Object -Last 1).id
if ($SupportsAfterPagination -and (-not $UserRequestedPageSize) -and $Page.Count -gt 0 -and $NextAfter -and ($NextAfter -ne $AfterCursor)) {
Source/Private/New-NinjaOneGETRequest.ps1:180
- The new array pagination path is what drives the public
/v2/organizations,/v2/devices,/v2/locations, and/v2/organization/{id}/devicescommands, but the added tests only cover a non-paginated array and never assert that a supported endpoint sendsafter, aggregates multiple pages, and stops correctly. Add a real helper-level test for a supported resource (plus the explicitpageSizeopt-out) so this high-impact branch is verified.
if ($SupportsAfterPagination -and (-not $UserRequestedPageSize) -and $Page.Count -gt 0 -and $NextAfter -and ($NextAfter -ne $AfterCursor)) {
$AfterCursor = $NextAfter
} else {
$FetchNextPage = $false
}
Source/Private/New-NinjaOneGETRequest.ps1:162
- For
/v2/custom-fields, the checked-in API contract points toSupportPaginationResponse, which exposespage,pageSize,count, andresultsbut nocursor.name(ninjaOne-API-core-resources.yaml:2203-2233and11264-11290). With that documented response,$NextCursoris always null, so an unboundedGet-NinjaOneCustomFieldsSchemacall still returns only the first page despite the newcursorNameplumbing. Implement the endpoint's actual continuation contract (or update the contract and validate the real response) before claiming this endpoint is automatically paged.
$NextCursor = $Result.cursor.name
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
…nchor-based pagination and update documentation for version 2.9.0
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Add organization-scoped locations to the pagination allow-list.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
Source/Private/New-NinjaOneGETRequest.ps1:64
Get-NinjaOneLocationsexposes-afterfor thev2/organization/{id}/locationsresource, but this allow-list only matches the top-levelv2/locations. Organization-scoped location responses therefore stop after the first page, so automatic pagination is incomplete. Include the organization-scoped locations resource in this pattern (as is already done for organization devices).
$SupportsAfterPagination = $resource -match '^/?v2/(organizations|organizations-detailed|devices|devices-detailed|locations|organization/[^/]+/devices)$'
Source/Private/New-NinjaOneGETRequest.ps1:61
- These continuation variables are reset to
$nulleven when the caller already suppliedcursor,olderThan,after, or an anchor in$QueryStringCollection. If the first response returns the same token/ID (a valid non-advancing cursor), the comparison below treats it as new, sends the same request again, and appends that page twice before stopping. Initialize the state from the corresponding input query values so the first page is compared with the token that was actually requested.
$Cursor = $null
$OlderThanCursor = $null
$AfterCursor = $null
$AnchorCursor = $null
Source/Public/PSModule/Invoke/Invoke-NinjaOneRequest.ps1:90
- This treats every GET response with
Content-Type: text/htmlas a rate-limit page, butGet-NinjaOneDeviceDashboardURL -redirectintentionally calls this function with-Rawand the endpoint returns an HTML redirect page. That existing command will now sleep/retry and eventually throw instead of returning the requested raw response. Exclude raw requests from this heuristic (or explicitly exempt the dashboard redirect endpoint) while retaining retries for normal JSON GETs.
$IsRateLimitedResponse = ($ContentType -match 'text/html') -or ($TrimmedContent -match '^(?i)<(!DOCTYPE|html)')
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
…e metadata validation logic
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Address the two moderate pagination and HTML-response classification issues before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Source/Private/New-NinjaOneGETRequest.ps1:190
- The same cursor corruption affects paginated array responses that support
-ParseDateTime, notably ticket log entries: the response converter can turn anidin an epoch range into aDateTimebefore$NextAnchoris read, soanchorIdorafterreceives a date string instead of the numeric cursor. Preserve the raw identifier for cursor construction while returning the converted page.
$Page = @($Result)
$PageResults.AddRange($Page)
$NextAnchor = ($Page | Select-Object -Last 1).$ContinuationPropertyName
if ($SupportsAfterPagination -and (-not $UserRequestedPageSize) -and $Page.Count -gt 0 -and $NextAnchor -and ($NextAnchor -ne $AfterCursor)) {
$AfterCursor = $NextAnchor
} elseif ($AnchorParameterName -and (-not $UserRequestedPageSize) -and $Page.Count -gt 0 -and $NextAnchor -and ($NextAnchor -ne $AnchorCursor)) {
$AnchorCursor = $NextAnchor
- Files reviewed: 13/15 changed files
- Comments generated: 2
- Review effort level: Lite
…nse processing; add tests for cursor pagination and HTML response handling
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Numeric cursor tokens can be converted to dates before the next paginated request.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
DevOps/Build/RequiredModules.psd1:2
- These dependency declarations now require Pester 6.1.0, but the developer documentation still says the tests run with Pester 5.x (
docs/NinjaOne/development/index.mdx:94). Please update that documentation (and any related setup guidance) so the documented test environment matches the required version.
Pester = '6.1.0'
DevOps/Quality/test.ps1:1
- This changes the test runner to require Pester 6.1.0, but
docs/NinjaOne/development/index.mdx:94still tells contributors that tests run with Pester 5.x. Please update the developer documentation with this dependency change so the documented setup matches the runner.
#requires -Module PowerShellGet, @{ ModuleName = 'Pester'; RequiredVersion = '6.1.0' }
Source/Private/ConvertFrom-NinjaOneDateTime.ps1:43
- This case-insensitive suffix match also treats ordinary names such as
valid,invalid, andguidas identifier fields. For a property likevalidcontaining an ISO/epoch value (or a nested object/list), the early return skips the recursive conversion, so the helper no longer converts all date values as documented. Make the identifier match case-aware and limit it to actualid/idsor camel-case identifier names.
if ($propertyName -match '(?i)ids?$') {
- Files reviewed: 15/17 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+43
to
+45
| if ($propertyName -match '(?i)ids?$') { | ||
| return $value | ||
| } |
Co-authored-by: homotechsual <2288257+homotechsual@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No final review comments identify unresolved blockers.
Review details
Suppressed comments (6)
DevOps/Build/RequiredModules.psd1:2
- This changes the required test framework to Pester 6.1.0, but
docs/NinjaOne/development/index.mdx:94still says the tests run with Pester 5.x. Please update that setup documentation in the same change so contributors are not given an incorrect dependency/version.
Pester = '6.1.0'
Source/Private/ConvertFrom-NinjaOneDateTime.ps1:47
(?i)ids?$matches any property whose name ends inid/ids, not just identifiers (for example,valid,rapid, orinvalid). With-ParseDateTime, a date-valued property with one of those names is returned unchanged, and an array under it is not recursively converted. Restrict this guard to the module's identifier forms (exactid/idsor a case-sensitiveId/Idssuffix) and add a regression case for a non-identifier suffix.
if ($propertyName -match '(?i)ids?$') {
Source/Private/New-NinjaOneGETRequest.ps1:164
- When
Get-NinjaOneActivitiesis called with the supportednewerThancursor, this branch still derives and sends anolderThancursor on the next request while retainingnewerThan. Subsequent requests therefore combine two cursor directions and can truncate or return the wrong activity set. HandlenewerThanseparately (for example, continue with the correct boundary for that direction) or disable automatic paging when it is supplied.
$NextOlderThan = ($Page | Select-Object -Last 1).id
if ((-not $UserRequestedPageSize) -and $Page.Count -gt 0 -and $NextOlderThan -and ($NextOlderThan -ne $OlderThanCursor)) {
$OlderThanCursor = $NextOlderThan
Source/Private/New-NinjaOneGETRequest.ps1:175
- The API contract models
cursor.nameas a cursor identifier and exposes a separatecursor.offset(ninjaOne-API-core-resources.yaml:11280-11292). Comparing only the name to$Cursorassumes the name changes on every page; if the server advances the offset under the same named cursor, this becomes false on the second page even with non-empty results, truncating query/backup reports. Track progress using the cursor state (for example, name plus offset) or the documented terminal condition rather than treating a stable name as the end of pagination.
$NextCursor = $Result.cursor.name
if ((-not $UserRequestedPageSize) -and $Page.Count -gt 0 -and $NextCursor -and ($NextCursor -ne $Cursor)) {
$Cursor = $NextCursor
Source/Private/New-NinjaOneGETRequest.ps1:174
- PowerShell's
-necomparison is case-insensitive, but cursor tokens are opaque strings and may differ only by case. In that case this condition treats a new cursor as non-advancing and stops before retrieving the remaining page. Compare the cursor values ordinally/case-sensitively (while normalizing numeric values to strings) before deciding to stop.
if ((-not $UserRequestedPageSize) -and $Page.Count -gt 0 -and $NextCursor -and ($NextCursor -ne $Cursor)) {
docs/NinjaOne/index.mdx:22
- This exception says
/v2/custom-fieldsis single-page and has no continuation token, but the updatedGet-NinjaOneCustomFieldsSchemaexposes-cursorNameand passesCursorParameterName = 'cursorName'; the checked-in OpenAPI path also describes this endpoint as paginated and definescursorNameas the next-page input. Please align this warning with the endpoint contract (or correct the contract and remove the special pagination wiring if single-page behavior is intentional), otherwise the landing-page guidance is misleading.
The 2.9.0 release of the NinjaOne PowerShell module introduces automatic pagination for GET endpoints with a documented continuation token. This includes cursor, activity, `after`, and ticketing anchor pagination. If you want to opt out of this behaviour for a specific call, supply `-PageSize` for that endpoint; the module will return only the requested page. The valid page-size range is endpoint-specific, so check the individual cmdlet help for the supported range on that API call. The custom-fields schema endpoint currently returns the single page defined by the API contract because its response does not include a continuation token.
- Files reviewed: 15/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
…ting in dictionary and list handling
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The schema test suite can fail when processing the exported function without metadata; fix the null-metadata skip condition.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
DevOps/Quality/test.ps1:1
- This changes the test harness to require Pester 6.1.0, but
docs/NinjaOne/development/index.mdx:94still tells contributors that tests run with Pester 5.x. Please update that development documentation in the same change so the documented setup matches the enforced dependency.
#requires -Module PowerShellGet, @{ ModuleName = 'Pester'; RequiredVersion = '6.1.0' }
Source/Public/CustomFields/Get/Get-NinjaOneCustomFieldsSchema.ps1:46
/v2/custom-fieldsis documented inninjaOne-API-core-resources.yamlasSupportPaginationResponse, whoseresultsresponse has no continuation cursor. PassingCursorParameterName = 'cursorName'therefore cannot makeNew-NinjaOneGETRequestauto-page, so this cmdlet still returns only the first page by default even though its help says “with default pagination.” Please either implement a continuation mechanism for this endpoint or update the cmdlet help to describe the single-page behavior consistently with the new module documentation.
CursorParameterName = 'cursorName'
- Files reviewed: 15/17 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Migrate remaining unqualified Should -Invoke calls in Tests/NinjaOne.Public.Tests.ps1 to Pester\Should-Invoke.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
DevOps/Build/RequiredModules.psd1:2
- This changes the pinned test runner to Pester 6.1.0, but
docs/NinjaOne/development/index.mdx:94still says the tests run with Pester 5.x. Please update that development setup documentation so contributors do not install or troubleshoot against a different test framework version.
Pester = '6.1.0'
Source/NinjaOne.psd1:15
- Updating
ModuleVersionto 2.9.0 while leavingPrivateData.PSData.ReleaseNotesat## 2026-04-20makes the published manifest advertise stale release notes;CHANGELOG.md:7now defines the 2.9.0 release as 2026-08-21. Please update the manifest release notes to the 2.9.0 entry as part of this release.
ModuleVersion = '2.9.0'
- Files reviewed: 15/17 changed files
- Comments generated: 1
- Review effort level: Lite
…eOrganisationDocument, New-NinjaOnePolicy, and other tests to improve readability and consistency
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No final review comments identify unresolved issues blocking approval.
Review details
Suppressed comments (3)
DevOps/Build/RequiredModules.psd1:2
- This changes the bootstrap dependency to Pester 6.1.0, but
docs/NinjaOne/development/index.mdx:94still documents the test suite as running with Pester 5.x. Please update the developer documentation in the same change so the documented environment matches whattest.ps1now requires.
Pester = '6.1.0'
DevOps/Quality/test.ps1:1
- This pins the test runner to Pester 6.1.0, but
docs/NinjaOne/development/index.mdx:94still instructs contributors that tests use Pester 5.x. Following the documented setup can therefore install/use an incompatible major version; update the development documentation (and any related version references) with this dependency change.
#requires -Module PowerShellGet, @{ ModuleName = 'Pester'; RequiredVersion = '6.1.0' }
Source/Public/PSModule/Invoke/Invoke-NinjaOneRequest.ps1:90
- This detection only runs when
Invoke-WebRequestreturns normally. PowerShell raises anHttpResponseException/WebExceptionfor non-success statuses such as 403 or 503, so an HTML rate-limit page with one of those statuses bypasses the retry logic and is rethrown by the catch block. Inspect the response attached to the HTTP exception in the error path and apply the same HTML/signature retry handling there (or otherwise ensure the API's non-2xx rate-limit response is handled).
$Response = Invoke-WebRequest @WebRequestParams -Headers $AuthHeaders -ContentType 'application/json;charset=utf-8'
# NinjaOne signals rate limiting by returning an HTML page (not a 429). Only retry safe GET requests; mutating requests should surface the HTML response instead of retrying.
$ContentType = [String]$Response.Headers['Content-Type']
$TrimmedContent = ([String]$Response.Content).TrimStart()
$IsHtmlResponse = ($ContentType -match 'text/html') -or ($TrimmedContent -match '^(?i)<(!DOCTYPE|html)')
- Files reviewed: 15/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
… upgrade Pester version in documentation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Correct the CI coverage upload mappings before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
.github/workflows/ci.yml:295
- The
private-flagged upload includes all four suite reports, so Codecov will attribute core/public/docs coverage to the private flag and the suite breakdown becomes inaccurate. This upload should contain only the private coverage report.
files: .artifacts/CodeCoverage.core.xml,.artifacts/CodeCoverage.private.xml,.artifacts/CodeCoverage.public.xml,.artifacts/CodeCoverage.docs.xml
.github/workflows/ci.yml:315
- The
docs-flagged upload includes all four suite reports, so Codecov will attribute core/private/public coverage to the docs flag and the suite breakdown becomes inaccurate. This upload should contain only the docs coverage report.
files: .artifacts/CodeCoverage.core.xml,.artifacts/CodeCoverage.private.xml,.artifacts/CodeCoverage.public.xml,.artifacts/CodeCoverage.docs.xml
.github/workflows/ci.yml:305
- The
public-flagged upload includes all four suite reports, so Codecov will attribute core/private/docs coverage to the public flag and the suite breakdown becomes inaccurate. This upload should contain only the public coverage report.
files: .artifacts/CodeCoverage.core.xml,.artifacts/CodeCoverage.private.xml,.artifacts/CodeCoverage.public.xml,.artifacts/CodeCoverage.docs.xml
Source/Public/CustomFields/Get/Get-NinjaOneCustomFieldsSchema.ps1:48
- This endpoint is configured with
cursorName, butNew-NinjaOneGETRequestonly advancesresultsresponses from$Result.cursor.name; the API schema'sSupportPaginationResponsefor/v2/custom-fieldshas nocursorproperty. Consequently a multi-page custom-field response is reduced to its first page while the help advertises pagination. Please either implement the endpoint's actual continuation-token extraction (with a regression test) or remove the automatic-pagination claim/parameter until the response contract supports it.
$RequestParams = @{
Resource = $Resource
QSCollection = $QSCollection
CursorParameterName = 'cursorName'
}
$CustomFieldsSchemaResults = New-NinjaOneGETRequest @RequestParams
- Files reviewed: 16/18 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot stopped reviewing on behalf of
homotechsual due to an error
August 24, 2026 06:32
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.
Introduce automatic pagination for API responses and implement graceful handling of HTML rate-limit responses. The changes enhance user experience by allowing seamless retrieval of multi-page results while providing options to opt-out of pagination. Update the module version to 2.9.0 and reflect these changes in the changelog.