-
Notifications
You must be signed in to change notification settings - Fork 5.5k
18770 hex #18832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
18770 hex #18832
Conversation
- Added prop definitions for project, run, user, group, and connection IDs in hex.app.mjs. - Introduced new actions: create, edit, delete groups; create, update, list data connections; run projects; and get project runs and run status. - Added utility functions for parsing objects and constants for sharing options and run statuses. - Updated package version to 0.1.0 and added dependencies. - Enhanced documentation links for actions and methods.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a full Hex integration: an API client with request helpers and paginators, propDefinitions for dynamic options, 11 new action modules for projects/users/groups/data connections, shared constants and a parseObject utility, and a package.json version/dependency update. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Workflow
participant Action as Action Module
participant App as hex.app.mjs
participant API as Hex API
rect rgb(245,250,255)
note right of Action: Build payload\n(parseObject for nested JSON)
User->>Action: Invoke action with props
Action->>App: this.hex.<endpoint>({ ..., $ })
App->>App: Build URL & headers\napply pagination if needed
App->>API: HTTP request
API-->>App: Response
App-->>Action: Return data
Action->>User: Export summary + return response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code graph analysis (1)components/hex/actions/list-projects/list-projects.mjs (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (3)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (9)
components/hex/hex.app.mjs (1)
356-381: Consider refining pagination termination condition.Line 378 sets
hasMore = runs.length, which continues pagination as long as any runs are returned. While this works correctly (pagination stops when an empty array is returned), it may result in one extra API call when the total results are an exact multiple of LIMIT.Consider this more explicit termination condition:
- hasMore = runs.length; + hasMore = runs.length === LIMIT;This stops pagination when fewer than LIMIT results are returned, avoiding the extra API call.
components/hex/actions/get-run-status/get-run-status.mjs (1)
9-12: Mark this action as read‑onlyThis performs a GET; set readOnlyHint to true for accurate metadata.
annotations: { destructiveHint: false, openWorldHint: true, - readOnlyHint: false, + readOnlyHint: true, },components/hex/actions/list-projects/list-projects.mjs (3)
10-14: Mark as read‑onlyThis lists data only; set readOnlyHint to true.
annotations: { destructiveHint: false, openWorldHint: true, - readOnlyHint: false, + readOnlyHint: true, },
93-99: Add a sensible default for maxResultsAlign with other list actions; default to 100 results unless overridden.
maxResults: { type: "integer", label: "Max Results", description: "The maximum number of results to return.", + default: 100, optional: true, },
132-134: Harden error handlingresponse or response.data.reason may be undefined; guard and surface a useful message.
- } catch ({ response }) { - throw new ConfigurationError(response.data.reason); - } + } catch (err) { + const reason = + err?.response?.data?.reason + || err?.response?.data?.message + || err?.response?.statusText + || err?.message + || "Unknown error"; + throw new ConfigurationError(reason); + }components/hex/actions/create-group/create-group.mjs (1)
39-41: Make users mapping resilient to both string IDs and objectsSupport either "abc123" or { id: "abc123" } inputs.
- users: this.users && parseObject(this.users)?.map((user) => ({ - id: user, - })), + users: this.users && parseObject(this.users)?.map((u) => ({ + id: typeof u === "string" ? u : (u?.id ?? u), + })),components/hex/actions/create-data-connection/create-data-connection.mjs (1)
35-40: Fix incomplete suggested payload structure — addaccessfield to match API schemaThe
groupsprop is correctly identified as unused. The Hex API acceptssharing.groups, but the suggested Option A is incomplete. The API requires objects with bothidandaccessfields, not justid. Update the suggestion to:data: { sharing: { workspace: { public: this.sharingWorkspacePublic, guests: this.sharingWorkspaceGuests, members: this.sharingWorkspaceMembers, }, + groups: this.groups && parseObject(this.groups)?.map((item) => ({ id: item.id, access: "MEMBER" })), },(Adjust the access value as needed per your use case; valid values: "NONE", "MEMBER", etc.)
Alternatively, remove the
groupsprop if this feature is not currently supported.components/hex/actions/update-group/update-group.mjs (1)
44-51: Consider conditionally including optional fields.Both
nameandusersare optional, but they're always included in the data payload (even asundefined). Consider only including them when provided to avoid sending unnecessary fields to the API.Apply this diff to conditionally include fields:
data: { - name: this.name, - members: { - users: this.users && parseObject(this.users)?.map((user) => ({ - id: user, - })), - }, + ...(this.name && { name: this.name }), + ...(this.users && { + members: { + users: parseObject(this.users)?.map((user) => ({ + id: user, + })), + }, + }), },components/hex/actions/edit-data-connection/edit-data-connection.mjs (1)
108-126: Consider making more fields optional and conditionally include them.For an edit operation, users typically want to update specific fields without affecting others. Currently, all fields are included in the payload (even as
undefined), which may cause unintended side effects depending on API behavior. Consider marking more props as optional and conditionally including them only when provided.
📜 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 (15)
components/hex/actions/create-data-connection/create-data-connection.mjs(1 hunks)components/hex/actions/create-group/create-group.mjs(1 hunks)components/hex/actions/deactivate-user/deactivate-user.mjs(1 hunks)components/hex/actions/delete-group/delete-group.mjs(1 hunks)components/hex/actions/edit-data-connection/edit-data-connection.mjs(1 hunks)components/hex/actions/get-project-runs/get-project-runs.mjs(1 hunks)components/hex/actions/get-run-status/get-run-status.mjs(1 hunks)components/hex/actions/list-data-connections/list-data-connections.mjs(1 hunks)components/hex/actions/list-projects/list-projects.mjs(1 hunks)components/hex/actions/run-project/run-project.mjs(1 hunks)components/hex/actions/update-group/update-group.mjs(1 hunks)components/hex/common/constants.mjs(1 hunks)components/hex/common/utils.mjs(1 hunks)components/hex/hex.app.mjs(1 hunks)components/hex/package.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
components/hex/actions/get-project-runs/get-project-runs.mjs (1)
components/hex/common/constants.mjs (2)
RUN_STATUS_OPTIONS(28-34)RUN_STATUS_OPTIONS(28-34)
components/hex/common/constants.mjs (1)
components/hex/hex.app.mjs (1)
LIMIT(38-44)
components/hex/actions/delete-group/delete-group.mjs (3)
components/hex/actions/create-group/create-group.mjs (1)
response(34-44)components/hex/actions/deactivate-user/deactivate-user.mjs (1)
response(26-29)components/hex/actions/update-group/update-group.mjs (1)
response(41-52)
components/hex/actions/list-data-connections/list-data-connections.mjs (2)
components/hex/actions/get-project-runs/get-project-runs.mjs (1)
response(39-47)components/hex/actions/list-projects/list-projects.mjs (2)
response(102-120)results(122-122)
components/hex/actions/deactivate-user/deactivate-user.mjs (1)
components/hex/actions/delete-group/delete-group.mjs (1)
response(24-27)
components/hex/actions/create-data-connection/create-data-connection.mjs (2)
components/hex/actions/edit-data-connection/edit-data-connection.mjs (1)
response(105-127)components/hex/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/hex/actions/list-projects/list-projects.mjs (3)
components/hex/actions/get-project-runs/get-project-runs.mjs (1)
response(39-47)components/hex/actions/list-data-connections/list-data-connections.mjs (1)
response(44-52)components/hex/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/hex/actions/edit-data-connection/edit-data-connection.mjs (2)
components/hex/actions/create-data-connection/create-data-connection.mjs (1)
response(103-125)components/hex/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/hex/hex.app.mjs (2)
components/hex/common/constants.mjs (10)
LIMIT(1-1)LIMIT(1-1)SHARING_WORKSPACE_PUBLIC_OPTIONS(3-6)SHARING_WORKSPACE_PUBLIC_OPTIONS(3-6)SHARING_WORKSPACE_GUESTS_OPTIONS(8-12)SHARING_WORKSPACE_GUESTS_OPTIONS(8-12)SCHEMA_REFRESH_ACCESS_OPTIONS(14-17)SCHEMA_REFRESH_ACCESS_OPTIONS(14-17)TYPE_OPTIONS(19-26)TYPE_OPTIONS(19-26)components/hex/actions/get-project-runs/get-project-runs.mjs (1)
runs(49-49)
components/hex/actions/create-group/create-group.mjs (2)
components/hex/actions/update-group/update-group.mjs (1)
response(41-52)components/hex/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/hex/actions/get-run-status/get-run-status.mjs (3)
components/hex/actions/deactivate-user/deactivate-user.mjs (1)
response(26-29)components/hex/actions/delete-group/delete-group.mjs (1)
response(24-27)components/hex/actions/run-project/run-project.mjs (1)
response(64-75)
components/hex/actions/update-group/update-group.mjs (2)
components/hex/actions/create-group/create-group.mjs (1)
response(34-44)components/hex/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
🔇 Additional comments (10)
components/hex/package.json (1)
3-3: LGTM! Appropriate version bump and dependency addition.The version bump to 0.1.0 correctly reflects the addition of new features, and the @pipedream/platform dependency aligns with the platform imports used across the action modules.
Also applies to: 15-17
components/hex/common/constants.mjs (1)
1-34: LGTM! Well-organized constants.The constants are properly defined and provide consistent option sets for the Hex integration actions.
components/hex/hex.app.mjs (2)
1-9: LGTM! Well-structured API client implementation.The API client methods, propDefinitions, and cursor-based pagination implementation follow best practices and provide a comprehensive interface to the Hex API.
Also applies to: 13-30, 48-219, 234-310, 327-355
31-47: Offset-based pagination for runId is correct.Hex's runs endpoints support offset/limit pagination with query parameters limit (integer 1–100) and offset (integer ≥0). The implementation at lines 40–43 correctly uses this approach, which differs from other props only because they target different API endpoints with different pagination requirements. No changes needed.
components/hex/actions/deactivate-user/deactivate-user.mjs (1)
1-37: LGTM! Proper error handling implemented.The action correctly implements error handling with try/catch and ConfigurationError, providing clear feedback to users.
components/hex/actions/run-project/run-project.mjs (1)
1-84: LGTM! Comprehensive project run configuration.The action provides thorough configuration options for running Hex projects with proper error handling.
components/hex/common/utils.mjs (1)
1-24: LGTM! Robust parseObject utility.The function correctly handles various input types with appropriate error handling for JSON parsing failures.
components/hex/actions/get-run-status/get-run-status.mjs (1)
32-41: LGTM on core logicProp wiring and API call pattern match existing actions; summary is clear.
components/hex/actions/get-project-runs/get-project-runs.mjs (1)
38-59: LGTM on pagination loopUse of paginateProjectRuns + async iteration and summary pluralization looks good.
components/hex/actions/create-group/create-group.mjs (1)
34-47: LGTMAPI call wiring and summary look correct.
components/hex/actions/create-data-connection/create-data-connection.mjs
Outdated
Show resolved
Hide resolved
components/hex/actions/edit-data-connection/edit-data-connection.mjs
Outdated
Show resolved
Hide resolved
components/hex/actions/edit-data-connection/edit-data-connection.mjs
Outdated
Show resolved
Hide resolved
…ection.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…on.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ions.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Removed the 'groups' prop definition from edit-data-connection.mjs. - Updated the name of the action in get-project-runs.mjs from "Run Project Runs" to "Get Project Runs".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @luancazarine lgtm! Ready for QA!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
components/hex/actions/get-project-runs/get-project-runs.mjs (1)
9-13: Fix readOnlyHint annotation.The
readOnlyHintis still set tofalsebut this action performs a read-only GET operation. It should be marked astrue.Apply this diff:
annotations: { destructiveHint: false, openWorldHint: true, - readOnlyHint: false, + readOnlyHint: true, },
🧹 Nitpick comments (1)
components/hex/actions/list-projects/list-projects.mjs (1)
77-91: Consider extracting sort options to constants.The hardcoded options for
sortByandsortDirectioncould be extracted to a shared constants file for better maintainability, especially if used in other actions.
📜 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 (5)
components/hex/actions/create-data-connection/create-data-connection.mjs(1 hunks)components/hex/actions/edit-data-connection/edit-data-connection.mjs(1 hunks)components/hex/actions/get-project-runs/get-project-runs.mjs(1 hunks)components/hex/actions/list-data-connections/list-data-connections.mjs(1 hunks)components/hex/actions/list-projects/list-projects.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
components/hex/actions/list-projects/list-projects.mjs (3)
components/hex/actions/get-project-runs/get-project-runs.mjs (1)
response(39-47)components/hex/actions/list-data-connections/list-data-connections.mjs (1)
response(44-52)components/hex/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/hex/actions/list-data-connections/list-data-connections.mjs (1)
components/hex/actions/list-projects/list-projects.mjs (2)
response(102-119)results(121-121)
components/hex/actions/get-project-runs/get-project-runs.mjs (1)
components/hex/common/constants.mjs (2)
RUN_STATUS_OPTIONS(28-34)RUN_STATUS_OPTIONS(28-34)
components/hex/actions/edit-data-connection/edit-data-connection.mjs (2)
components/hex/actions/create-data-connection/create-data-connection.mjs (1)
response(103-125)components/hex/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/hex/actions/create-data-connection/create-data-connection.mjs (2)
components/hex/actions/edit-data-connection/edit-data-connection.mjs (1)
response(99-121)components/hex/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (9)
components/hex/actions/get-project-runs/get-project-runs.mjs (2)
15-37: LGTM!The props are well-structured with appropriate types, descriptions, and defaults. The use of
propDefinitionforprojectIdand constants for status options follows best practices.
38-60: LGTM!The run function correctly implements the pagination pattern, accumulates results, and provides clear feedback. The pluralization in the summary message is a nice touch.
components/hex/actions/list-data-connections/list-data-connections.mjs (1)
43-63: LGTM! The run method correctly returns the collected results.The pagination logic properly collects items into an array and returns that array. The summary message accurately reports the count.
components/hex/actions/edit-data-connection/edit-data-connection.mjs (4)
1-3: LGTM!Imports are clean and necessary for the action implementation.
4-14: LGTM!Metadata and annotations are appropriate for an edit data connection action.
15-97: LGTM!Props are well-structured and follow the propDefinition pattern consistently. The previously flagged unused
groupsprop has been removed.
98-126: LGTM!The run method correctly builds the update payload and the previously incorrect summary message has been fixed. The implementation follows the established pattern from similar actions.
components/hex/actions/create-data-connection/create-data-connection.mjs (2)
1-2: LGTM!Imports are correct and both are utilized in the action.
102-129: LGTM overall!The run method correctly constructs the payload and calls the Hex API. The use of
parseObjectfor nested structures is appropriate, and the summary export accurately reflects the created resource. However, ensure thegroupsprop concern (if applicable) is addressed.
components/hex/actions/create-data-connection/create-data-connection.mjs
Outdated
Show resolved
Hide resolved
components/hex/actions/list-data-connections/list-data-connections.mjs
Outdated
Show resolved
Hide resolved
components/hex/actions/list-data-connections/list-data-connections.mjs
Outdated
Show resolved
Hide resolved
- Removed unused 'groups' prop definition from create-data-connection action. - Updated 'readOnlyHint' to true in list-data-connections action. - Improved error handling in list-projects action to provide more informative error messages.
|
/approve |
Resolves #18770
Summary by CodeRabbit
New Features
Chores