Skip to content

Disable paging and raise pageSize on NinjaOne array-mode datastreams - #52

Open
Deenk wants to merge 5 commits into
mainfrom
work/fw/ninjaone-paging-fixes
Open

Disable paging and raise pageSize on NinjaOne array-mode datastreams#52
Deenk wants to merge 5 commits into
mainfrom
work/fw/ninjaone-paging-fixes

Conversation

@Deenk

@Deenk Deenk commented May 22, 2026

Copy link
Copy Markdown
Contributor

📋 Summary

The 6 datastreams that hit NinjaOne's array endpoints (/v2/devices, /v2/organizations, /v2/locations, plus the 3 scoped variants) were configured with token-mode paging reading metadata.next_page_token from the response. That field doesn't exist for these specific endpoints. NinjaOne returns a plain JSON array with no envelope or pagination headers, so paging silently stopped after page 1, capping users at 1000 devices.

As I understand it, the SquaredUp Web API plugin has no paging mode that fits NinjaOne's pattern of "last item's id as the next cursor" (verified against the API spec, live API response, and the Web API paging support doc). Fixing this properly needs a framework change I think.

As a stopgap, switch to mode: none and request pageSize: 10000 in getArgs. NinjaOne accepts arbitrary pageSize values without error in testing, though we couldn't verify the actual server-side cap on a small test tenant. This matches the precedent already used in activities.json.


🔗 Related issue(s)


🧩 Plugin details

  • Plugin name:
  • Type of change:
    • Bug fix
    • New datastream
    • Enhancement to existing datastream
    • Performance improvement
    • Documentation / metadata / logo
    • Other (please describe):

⚠️ Breaking changes

Does this PR introduce any breaking changes?

  • No
  • Yes (please describe):

If yes, describe:

  • What breaks
  • Who is impacted
  • Any migration steps

📚 Documentation

  • Documentation updated
  • No documentation changes needed

✅ Checklist

  • No secrets or credentials included
  • Plugin, datastream and UI naming follow SquaredUp guidelines
  • I agree to the Code of Conduct

Summary by CodeRabbit

  • Bug Fixes

    • Improved pagination for NinjaOne devices, locations, and organizations.
    • Prevented filtered results from causing records to be skipped across pages.
    • Improved handling of empty result pages and cursor-based navigation.
  • Chores

    • Updated the NinjaOne integration to version 1.1.15.

The 6 datastreams that hit NinjaOne's bare-array endpoints (/v2/devices,
/v2/organizations, /v2/locations, plus the 3 scoped variants) were configured
with token-mode paging reading metadata.next_page_token from the response.
That field doesn't exist - NinjaOne returns a plain JSON array with no
envelope or pagination headers - so paging silently stopped after page 1,
capping users at 1000 devices.

The SquaredUp Web API plugin has no paging mode that fits NinjaOne's pattern
of "last item's id as the next cursor" (verified against the API spec, live
API response, and the Web API paging support doc). Fixing this properly
needs a framework change.

As a stopgap, switch to mode: none and request pageSize: 10000 in getArgs.
NinjaOne accepts arbitrary pageSize values without error in testing, though
we couldn't verify the actual server-side cap on a small test tenant. This
matches the precedent already used in activities.json.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Deenk Deenk self-assigned this May 22, 2026
@Deenk Deenk added the bug Something isn't working label May 22, 2026
@clarkd

clarkd commented May 26, 2026

Copy link
Copy Markdown
Member

As I understand it, the SquaredUp Web API plugin has no paging mode that fits NinjaOne's pattern of "last item's id as the next cursor" (verified against the API spec, live API response, and the Web API paging support doc). Fixing this properly needs a framework change I think.

@WaynePlummer can advise here

@Deenk
Deenk marked this pull request as ready for review May 28, 2026 14:48
@Deenk
Deenk requested a review from a team May 28, 2026 14:48
Comment thread plugins/NinjaOne/v1/dataStreams/devices.json
@WaynePlummer

Copy link
Copy Markdown
Contributor

the API

Shame - @Deenk: can you point me at the API docs for this and I'll raise a JIRA for us to add support. Cheers

@Deenk

Deenk commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

the API

Shame - @Deenk: can you point me at the API docs for this and I'll raise a JIRA for us to add support. Cheers

Thanks @WaynePlummer , here are the API docs. openapi spec is at the link too.

https://app.ninjarmm.com/apidocs/

@clarkd

clarkd commented Jun 8, 2026

Copy link
Copy Markdown
Member

I'll hold off on approving/merging this until @WaynePlummer can confirm this is the best approach right now? It seems a shame to lose the paging support here.

@clarkd

clarkd commented Jun 17, 2026

Copy link
Copy Markdown
Member

@claude review once

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things still outstanding: the version bump I flagged previously hasn't landed (metadata.json is still at 1.1.9), and clarkd's question to WaynePlummer about whether dropping paging entirely is the right stopgap doesn't appear to have been answered yet.

Extended reasoning...

Overview

This PR switches 6 NinjaOne data streams (devices, locations, organizations, and the 3 scoped variants) from paging.mode: token (reading a non-existent metadata.next_page_token) to paging.mode: none with pageSize: 10000 in getArgs. The fix is small, self-contained, and matches the precedent in activities.json. The diff is mechanical and the JSON edits look correct in all 6 files.

Security risks

None. These are JSON config edits to data-stream paging behavior; no auth/crypto/credential handling is touched, and no scripts changed.

Level of scrutiny

Low intrinsic complexity (config-only), but this is the second review pass on the same revision since I commented on 2026-05-28. Nothing substantive has changed in the working tree since then.

Other factors

  1. Version bump still missing. plugins/NinjaOne/v1/metadata.json still reads 1.1.9. The repo's review guidance ("Any diff that touches files inside a plugin directory must include a corresponding change to metadata.json that increases the version field. If no version bump is present, assume the task is unfinished") is unambiguous. My previous inline comment on this is still unresolved.
  2. Open reviewer question. clarkd (2026-06-08) explicitly held off pending WaynePlummer's input on whether the stopgap (lose all paging, request pageSize=10000) is the right approach, given the author themselves notes the server-side cap couldn't be verified on a small test tenant. WaynePlummer hasn't weighed in on that question yet — only on raising a JIRA for framework support.
  3. Behavioral risk worth a human eye. If a tenant has more than 10000 devices/orgs/locations and the NinjaOne server enforces a smaller cap, this stopgap silently truncates results just as the previous token-mode bug did — different failure mode, same user-visible symptom. A human reviewer (and ideally WaynePlummer) should sign off that this trade-off is acceptable as a stopgap.

@WaynePlummer

WaynePlummer commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

I'll hold off on approving/merging this until @WaynePlummer can confirm this is the best approach right now? It seems a shame to lose the paging support here.

So, it's not actually documented anywhere, but if you configure a post-request script for the data stream, it can set the special variable pagingContext to whatever you like. If you have configured paging with mode "Token", the configuration specified in "Token in response" would be immaterial as your script would be setting the correct value from the last row in the payload. The configuration for "Token in request" would still pertain.

Your script could be as simple as:

result = data;
pagingContext = data[data.length - 1].id;

@clarkd

clarkd commented Jun 22, 2026

Copy link
Copy Markdown
Member

I'll hold off on approving/merging this until @WaynePlummer can confirm this is the best approach right now? It seems a shame to lose the paging support here.

So, it's not actually documented anywhere, but if you configure a post-request script for the data stream, it can set the special variable pagingContext to whatever you like. If you have configured paging with mode "Token", the configuration specified in "Token in response" would be immaterial as your script would be setting the correct value from the last row in the payload. The configuration for "Token in request" would still pertain.

Your script could be as simple as:

result = data;
pagingContext = data[data.length - 1].id;

Any thoughts @Deenk?

@Deenk

Deenk commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

I'll hold off on approving/merging this until @WaynePlummer can confirm this is the best approach right now? It seems a shame to lose the paging support here.

So, it's not actually documented anywhere, but if you configure a post-request script for the data stream, it can set the special variable pagingContext to whatever you like. If you have configured paging with mode "Token", the configuration specified in "Token in response" would be immaterial as your script would be setting the correct value from the last row in the payload. The configuration for "Token in request" would still pertain.
Your script could be as simple as:

result = data;
pagingContext = data[data.length - 1].id;

Any thoughts @Deenk?

Thanks @clarkd @WaynePlummer

I will try and implement this approach this week now I'm back from holiday

@clarkd

clarkd commented Aug 17, 2026

Copy link
Copy Markdown
Member

Hi @Deenk - could you confirm any plans for this one, otherwise we'll have to close as stale. LMK.

@Deenk

Deenk commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Hi @Deenk - could you confirm any plans for this one, otherwise we'll have to close as stale. LMK.

I just realised this is still open, oops!

I will get this resolved today and re-review.

Deenk and others added 2 commits August 24, 2026 09:59
Restores real token paging (mode: token, cursor via after=<id> query
arg) for devices/locations/organizations streams instead of the
mode:none + 10000-row cap stopgap. NinjaOne's bare-array endpoints have
no next-page token field, so each stream's postRequestScript now sets
pagingContext directly from the last item's id in the raw response
page, which the base WebAPI plugin uses in place of paging.in.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: b646a740-51aa-423e-a225-480f7ec2985f

📥 Commits

Reviewing files that changed from the base of the PR and between 4aff6af and 6475558.

📒 Files selected for processing (1)
  • plugins/NinjaOne/v1/metadata.json

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Walkthrough

Walkthrough

NinjaOne data streams now support cursor pagination for bare-array responses. Cursors use the final raw item ID, including when output filtering occurs. Empty pages clear the cursor. The plugin metadata version increases to 1.1.15.

Changes

NinjaOne pagination

Layer / File(s) Summary
Raw response cursor handling
plugins/NinjaOne/v1/dataStreams/scripts/{devices,locations,organizations}.js, plugins/NinjaOne/v1/metadata.json
Device, location, and organization streams derive pagingContext from the final raw item ID and clear it for empty pages. The plugin version changes to 1.1.15.
Filtered stream pagination
plugins/NinjaOne/v1/dataStreams/scripts/{locationDevices,organizationLocations}.js
Filtered streams derive pagingContext from the final item in the unfiltered API response.

Merge Risk: ⚪ Minimal · up to 64755

The PR disables incompatible token paging and raises the requested page size for NinjaOne array datastreams to prevent results from stopping at the first page; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title describes disabling paging and increasing pageSize, but the changes implement cursor paging through pagingContext. Rename the title to describe cursor-based paging for NinjaOne array-mode datastreams.
Description check ⚠️ Warning The description is structured and detailed, but it documents a stopgap that the changes did not implement and leaves the plugin name blank. Update the description to document the pagingContext cursor implementation and complete the plugin details section.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@Deenk

Deenk commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

@clarkd this is now ready to go/review. I have put in the requested changes and tested it myself. All data streams and imports still work.

@github-actions

Copy link
Copy Markdown

🧩 Plugin PR Summary

📦 Modified Plugins

  • plugins/NinjaOne/v1

📋 Results

Step Status
Scope & version ✅ Passed
Validation ✅ Passed
Deployment 🚀 Deployed

🔍 Validation Details

ninja-one
{
  "valid": true,
  "pluginName": "ninja-one",
  "pluginType": "cloud",
  "summary": {
    "Data Streams": 37,
    "Import Definitions": 1,
    "Correlation Rules": 0,
    "UI Configuration": true,
    "Has Icon": true,
    "Has Default Content": true,
    "Config Validation": true,
    "Custom Types": true
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working plugin: NinjaOne

Development

Successfully merging this pull request may close these issues.

3 participants