New Components - ninjaone #16755
New Components - ninjaone #16755
Conversation
luancazarine
added
the
ai-assisted
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
|
Warning Rate limit exceeded@luancazarine has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
""" WalkthroughThis update introduces a comprehensive NinjaOne integration, adding new actions for ticket creation and device updates, a polling source for device online events, utility and constants modules, and dynamic property definitions with API pagination. Minor formatting changes were also made to unrelated app files by adding trailing newlines. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NinjaOne Action (Create Ticket)
participant NinjaOne App
participant NinjaOne API
User->>NinjaOne Action (Create Ticket): Provide ticket details
NinjaOne Action (Create Ticket)->>NinjaOne App: Call createSupportTicket()
NinjaOne App->>NinjaOne API: POST /tickets with payload
NinjaOne API-->>NinjaOne App: Response (ticket ID)
NinjaOne App-->>NinjaOne Action (Create Ticket): Return result
NinjaOne Action (Create Ticket)-->>User: Output summary
sequenceDiagram
participant Polling Source
participant NinjaOne App
participant NinjaOne API
participant User
Polling Source->>NinjaOne App: listActivities({ newerThan: lastId })
NinjaOne App->>NinjaOne API: GET /activities
NinjaOne API-->>NinjaOne App: Return activities
NinjaOne App-->>Polling Source: Activities list
Polling Source->>User: Emit new device online events
sequenceDiagram
participant User
participant NinjaOne Action (Update Device)
participant NinjaOne App
participant NinjaOne API
User->>NinjaOne Action (Update Device): Provide device ID and update fields
NinjaOne Action (Update Device)->>NinjaOne App: Call updateDevice()
NinjaOne App->>NinjaOne API: PATCH /devices/{deviceId}
NinjaOne API-->>NinjaOne App: Response
NinjaOne App-->>NinjaOne Action (Update Device): Return result
NinjaOne Action (Update Device)-->>User: Output summary
Assessment against linked issues
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Sources - New Device Online (Instant) Actions - Create Ticket - Update Device
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
components/ninjaone/actions/update-device/update-device.mjs (1)
16-16: Fix trailing space in description.There's a trailing space at the end of the description that should be removed for consistency.
- description: "The Id of the device to update ", + description: "The Id of the device to update",components/ninjaone/common/constants.mjs (1)
1-23: Constants are well-defined and organized.The exported constants provide appropriate options for ticket classification and a reasonable pagination limit. The values align with standard ticket management system categories.
Consider adding a newline at the end of the file for consistency with standard formatting practices.
components/ninjaone/ninjaone.app.mjs (1)
33-47: Consider adding pagination support for consistency.The implementation is correct, but unlike other prop definitions, this one doesn't support pagination. If the number of ticket forms can grow large, consider adding pagination support for consistency and scalability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
components/heylibby/heylibby.app.mjs(1 hunks)components/neetocal/neetocal.app.mjs(1 hunks)components/neetodesk/neetodesk.app.mjs(1 hunks)components/ninjaone/actions/create-ticket/create-ticket.mjs(1 hunks)components/ninjaone/actions/update-device/update-device.mjs(1 hunks)components/ninjaone/common/constants.mjs(1 hunks)components/ninjaone/common/utils.mjs(1 hunks)components/ninjaone/ninjaone.app.mjs(1 hunks)components/ninjaone/package.json(2 hunks)components/ninjaone/sources/common/base.mjs(1 hunks)components/ninjaone/sources/new-device-online/new-device-online.mjs(1 hunks)components/ninjaone/sources/new-device-online/test-event.mjs(1 hunks)components/scrapecreators/scrapecreators.app.mjs(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
components/ninjaone/ninjaone.app.mjs
[error] 318-318: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (18)
components/neetocal/neetocal.app.mjs (1)
11-11: Trailing newline addition – purely formatting
This change adds a newline at EOF for consistency with other component files. No functional or logical code was modified.components/neetodesk/neetodesk.app.mjs (1)
11-11: Trailing newline addition – purely formatting
This change adds a newline at EOF for consistency with other component files. No functional or logical code was modified.components/heylibby/heylibby.app.mjs (1)
11-11: Trailing newline addition – purely formatting
This change adds a newline at EOF for consistency with other component files. No functional or logical code was modified.components/scrapecreators/scrapecreators.app.mjs (1)
11-11: Trailing newline addition – purely formatting
This change adds a newline at EOF for consistency with other component files. No functional or logical code was modified.components/ninjaone/package.json (1)
3-17: LGTM! Package configuration updated correctly.The version bump to 0.1.0 appropriately reflects the new feature additions, and the @pipedream/platform dependency is correctly added for the new NinjaOne components.
components/ninjaone/sources/new-device-online/test-event.mjs (1)
1-18: Well-structured test event data.The test event properly represents a device reboot event with realistic data structure, appropriate timestamp formatting, and nested message parameters that would be typical for NinjaOne API responses.
components/ninjaone/actions/update-device/update-device.mjs (1)
56-71: Action implementation looks solid.The run method properly destructures props, makes the API call with correct parameters, and provides informative feedback. The error handling is delegated to the underlying ninjaone app method, which is appropriate.
components/ninjaone/common/utils.mjs (1)
1-24: LGTM! Robust JSON parsing utility.The
parseObjectfunction handles various input types well with proper error handling for JSON parsing failures. The defensive programming approach with falsy checks and type validation is excellent.components/ninjaone/sources/common/base.mjs (2)
56-64: Good deployment and execution pattern.The deploy hook with a 25-event limit for initial setup and unlimited polling in the run method follows best practices for polling sources.
22-54:❓ Verification inconclusive
Verify the API response ordering assumption.
The implementation assumes the API returns activities in newest-first order (evidenced by setting
lastIdtoresponseArray[0].idand then reversing before emission). Please ensure this matches the actual NinjaOne API behavior.Run this script to verify the API response ordering:
🏁 Script executed:
#!/bin/bash # Description: Search for NinjaOne API documentation or implementation details about activity ordering # Look for any documentation about listActivities method ordering rg -A 10 -B 5 "listActivities|activity.*order|newer.*than" --type js # Search for any API response handling that might indicate ordering ast-grep --pattern 'listActivities($$$) { $$$ }'Length of output: 123
Verify the NinjaOne API response ordering assumption
I wasn’t able to locate any definitive reference to the ordering of
listActivitiesin the codebase. Before relying onresponseArray[0].idas the newest event, please:
- Check the official NinjaOne API documentation (or contact support) to confirm that
listActivitiesreturns activities sorted newest-first.- If it actually returns oldest-first, update the
lastIdassignment to use the last element in the array instead.Ensuring this aligns with the API will prevent missing or duplicating events in your polling logic.
components/ninjaone/sources/new-device-online/new-device-online.mjs (1)
4-27: Well-structured source component following good extension patterns.The implementation properly extends the base polling source with appropriate filtering for device reboot events. The method overrides are clean and focused on the specific use case.
components/ninjaone/actions/create-ticket/create-ticket.mjs (2)
19-141: Excellent prop definition structure leveraging dynamic options.The comprehensive use of
propDefinitionfor dynamic options and the logical grouping of ticket properties demonstrates good component design patterns.
142-180: Robust ticket creation logic with proper error handling.The implementation properly structures the nested data objects, uses utility functions for data parsing, and provides clear error messaging through the ConfigurationError pattern.
components/ninjaone/ninjaone.app.mjs (5)
1-3: LGTM!The import statements are clean and properly structured.
8-32: LGTM!The
clientIdprop definition correctly implements pagination and maps organization data to options.
48-75: LGTM!The
locationIdprop definition correctly implements organization-specific filtering and pagination.
134-152: LGTM!The
assignedAppUserIdprop definition correctly filters for technicians and provides descriptive labels.
210-226: LGTM!The base utility methods follow best practices for API client implementation.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
components/ninjaone/sources/new-device-online/new-device-online.mjs
Outdated
Show resolved
Hide resolved
…e.mjs Co-authored-by: michelle0927 <[email protected]>
Co-authored-by: michelle0927 <[email protected]>
Co-authored-by: michelle0927 <[email protected]>
Co-authored-by: michelle0927 <[email protected]>
Co-authored-by: michelle0927 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
components/ninjaone/ninjaone.app.mjs (2)
104-118:⚠️ Potential issueFix missing pagination context in tags options (duplicate issue).
This is the same issue flagged in previous reviews that remains unresolved. The
tagsprop definition returns only the tag names array without the pagination context, which will break pagination functionality.Apply this fix to include the pagination context:
tags: { type: "string[]", label: "Tags", description: "A list of tags related to ticket.", async options({ prevContext }) { const { tags } = await this.listTags({ params: { pageSize: LIMIT, after: prevContext.lastId, }, }); - return tags.map(({ name }) => name); + return { + options: tags.map(({ name }) => name), + context: { + lastId: tags[tags.length - 1]?.id || tags[tags.length - 1]?.name, + }, + }; }, },
307-336:⚠️ Potential issueFix critical issues in the paginate method (duplicate issue).
This is the same set of issues flagged in previous reviews that remain unresolved. The paginate method has several critical problems:
- The
deleteoperator impacts performance (line 318) - also flagged by static analysis- The
lastIdvariable is never updated in the loop, breaking pagination- The
hasMorecondition logic is incorrectApply this fix to resolve all issues:
async *paginate({ fn, params = {}, maxResults = null, ...opts }) { let hasMore = false; let count = 0; let lastId = 0; const newerThan = params.newerThan; do { if (lastId) { params.olderThan = lastId; - if (params.newerThan) delete params.newerThan; + params.newerThan = undefined; } const { activities } = await fn({ params, ...opts, }); for (const d of activities) { yield d; + lastId = d.id; if (maxResults && ++count === maxResults) { return count; } } - hasMore = activities.length && (lastId && (lastId > newerThan)); + hasMore = activities.length > 0; } while (hasMore); },🧰 Tools
🪛 Biome (1.9.4)
[error] 318-318: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🧹 Nitpick comments (2)
components/ninjaone/ninjaone.app.mjs (2)
10-11: Improve description consistency.The description has inconsistent capitalization ("Id" vs "ID") and could be clearer.
Apply this improvement:
- label: "Client Id", - description: "The Id of the client related to the ticket", + label: "Client ID", + description: "The ID of the client related to the ticket",
78-79: Fix awkward phrasing in description.The description "The ID of the device to the ticket" has awkward phrasing.
Apply this improvement:
- description: "The ID of the device to the ticket", + description: "The ID of the device related to the ticket",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/ninjaone/ninjaone.app.mjs(1 hunks)components/ninjaone/sources/new-device-online/new-device-online.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/ninjaone/sources/new-device-online/new-device-online.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/ninjaone/ninjaone.app.mjs
[error] 318-318: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (2)
components/ninjaone/ninjaone.app.mjs (2)
246-261: API paths have been corrected.The missing leading slashes in
listLocationsandlistDevicesmethods have been properly fixed. The API paths now correctly include the leading slash.
1-339: Overall implementation is well-structured but requires critical bug fixes.The NinjaOne app component demonstrates good architectural patterns with proper API client structure, consistent method naming, and comprehensive prop definitions. However, critical issues identified in previous reviews remain unresolved and must be addressed before this can be considered production-ready.
Priority fixes needed:
- Tags pagination functionality is broken
- Paginate method has multiple critical bugs affecting pagination logic
- Minor description improvements for consistency
The foundation is solid, but these bugs will cause runtime failures in pagination scenarios.
🧰 Tools
🪛 Biome (1.9.4)
[error] 318-318: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Co-authored-by: michelle0927 <[email protected]>
Co-authored-by: michelle0927 <[email protected]>
|
/approve |
Resolves #14845.
Summary by CodeRabbit
New Features
Chores