[Components] Metabase - new components #17008
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a comprehensive Metabase integration, adding a new app component with methods for interacting with the Metabase API. Multiple action modules are implemented, enabling dashboard and database retrieval and creation, as well as running queries. Utilities for JSON parsing are added, documentation is expanded, and the package configuration is updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant MetabaseApp
participant MetabaseAPI
User->>Action: Trigger (e.g., Run Query, Create Dashboard)
Action->>MetabaseApp: Call corresponding method (e.g., runCardQuery, createDashboard)
MetabaseApp->>MetabaseAPI: Send HTTP request (REST API)
MetabaseAPI-->>MetabaseApp: Return response
MetabaseApp-->>Action: Return data
Action-->>User: Output result/summary
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/metabase/actions/get-database/get-database.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/metabase/actions/create-dashboard/create-dashboard.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/metabase/metabase.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
components/metabase/actions/get-database/get-database.mjs (1)
25-46: Consider refactoring the metadata retrieval approach.While the logic is sound, accessing
app._makeRequest()(a seemingly private method indicated by the underscore prefix) is a code smell. Consider adding a dedicated public method likegetDatabaseMetadata()to the app for better encapsulation.if (includeMetadata) { - response = await this.app._makeRequest({ + response = await this.app.getDatabaseMetadata({ $, - path: `/database/${databaseId}/metadata`, + databaseId, }); }This would improve the API design and maintain better separation of concerns.
components/metabase/actions/create-card/create-card.mjs (1)
60-69: Consider adding input validation for the dataset_query structure.The data construction correctly maps inputs to API fields, but there's no validation to ensure the query object contains the required structure for either native or structured queries.
Add validation before constructing the dataset_query:
+ // Validate query structure + if (!query.native && !query.query) { + throw new Error("Query must contain either 'native' or 'query' property"); + } + const data = { name, database_id: databaseId, dataset_query: { database: databaseId, ...query, }, display: visualizationType || "table", visualization_settings: {}, };components/metabase/actions/create-dashboard/create-dashboard.mjs (1)
29-34: Enhance parameters prop documentation.The parameters prop description is too generic. Consider providing examples of the expected structure for dashboard parameters.
parameters: { type: "object", label: "Parameters", - description: "Dashboard parameters configuration", + description: "Dashboard parameters configuration. Example: [{\"id\": \"param1\", \"name\": \"Category\", \"slug\": \"category\", \"type\": \"category\"}]", optional: true, },components/metabase/metabase.app.mjs (3)
6-67: Consider performance optimizations for async option loaders.The property definitions are well-structured, but consider these improvements:
- Add error handling in async options methods to prevent crashes if API calls fail
- Consider pagination for large datasets - some Metabase instances may have hundreds of cards/dashboards
- Add caching or debouncing to avoid repeated API calls when users interact with dropdowns
The fallback handling for collection names (line 50) is good defensive programming.
Consider adding error handling like this:
async options() { + try { const databases = await this.getDatabases(); return databases.map((database) => ({ label: database.name, value: database.id, })); + } catch (error) { + console.error("Failed to load databases:", error); + return []; + } },
105-134: Consider pagination for large datasets.The entity listing methods are correctly implemented. For production use with large Metabase instances, consider adding pagination support as the API may return limited results by default.
You could add pagination parameters like:
getDatabases(args = {}) { return this._makeRequest({ path: "/database", + params: { + limit: args.limit || 50, + offset: args.offset || 0, + ...args.params, + }, ...args, }); },
135-158: Add input validation for ID parameters.The individual entity retrieval methods are well-structured. Consider adding input validation for ID parameters to prevent potential security issues if IDs come from untrusted sources.
Add validation like:
getCard({ cardId, ...args } = {}) { + if (!cardId || !Number.isInteger(Number(cardId))) { + throw new Error("Invalid cardId: must be a valid integer"); + } return this._makeRequest({ path: `/card/${cardId}`, ...args, }); },
📜 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 (11)
components/metabase/.gitignore(0 hunks)components/metabase/README.md(1 hunks)components/metabase/actions/create-card/create-card.mjs(1 hunks)components/metabase/actions/create-dashboard/create-dashboard.mjs(1 hunks)components/metabase/actions/get-dashboard/get-dashboard.mjs(1 hunks)components/metabase/actions/get-database/get-database.mjs(1 hunks)components/metabase/actions/run-query/run-query.mjs(1 hunks)components/metabase/app/metabase.app.ts(0 hunks)components/metabase/common/constants.mjs(1 hunks)components/metabase/metabase.app.mjs(1 hunks)components/metabase/package.json(1 hunks)
💤 Files with no reviewable changes (2)
- components/metabase/.gitignore
- components/metabase/app/metabase.app.ts
⏰ 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 (16)
components/metabase/package.json (2)
3-3: LGTM! Appropriate version bump for new features.The version update to 0.1.0 properly reflects the addition of new Metabase actions and follows semantic versioning conventions.
5-5: LGTM! Simplified main entry point.The change from "dist/app/metabase.app.mjs" to "metabase.app.mjs" simplifies the package structure and aligns with the new app implementation.
components/metabase/README.md (2)
5-14: Excellent documentation enhancement!The new "Available Actions" section provides users with a clear overview of the integration's capabilities. The list is comprehensive and well-organized.
23-25: Great addition of practical use cases.The new use cases demonstrate real-world applications for data pipeline automation and cross-platform integration, helping users understand the value proposition.
components/metabase/actions/run-query/run-query.mjs (2)
1-23: Well-structured action definition.The action follows Pipedream conventions with proper metadata, clear prop definitions, and good documentation linking to the official API docs.
24-39: Clean implementation with proper error handling delegation.The run method correctly extracts props, calls the app method with appropriate parameters, exports a helpful summary, and returns the response. The fallback to empty object for parameters is a good defensive programming practice.
components/metabase/actions/get-database/get-database.mjs (1)
1-24: Well-structured action definition with comprehensive props.The action properly defines metadata and props, including the useful
includeMetadataboolean flag for optional detailed information.components/metabase/actions/create-card/create-card.mjs (1)
71-77: LGTM! Clean optional field handling.The conditional logic for optional fields (collectionId and description) is well-implemented, avoiding undefined values in the API payload.
components/metabase/actions/get-dashboard/get-dashboard.mjs (1)
18-29: LGTM! Clean and robust implementation.The action correctly handles the dashboard retrieval with proper error context through the
$parameter. The optional chaining in the summary message (response.dashcards?.length || 0) is a good defensive programming practice.components/metabase/common/constants.mjs (2)
24-32: LGTM! Robust error parsing implementation.The error parsing function handles multiple error response formats gracefully, including message strings, error objects, and fallback to generic error messages. This comprehensive approach will improve error reporting across the integration.
34-46: LGTM! Clean utility functions with good defaults.The formatting functions provide consistent summary generation with proper default handling for optional properties like
dashcardsarray.components/metabase/actions/create-dashboard/create-dashboard.mjs (1)
49-55: LGTM! Consistent optional field handling pattern.The conditional logic for optional fields matches the pattern used in create-card action, maintaining consistency across the integration.
components/metabase/metabase.app.mjs (4)
1-5: LGTM! Standard Pipedream app structure.The imports and app definition follow the correct Pipedream conventions.
87-104: LGTM! HTTP verb methods are correctly implemented.The methods follow consistent patterns and properly delegate to the generic
_makeRequestmethod.
159-211: LGTM! CRUD operations follow REST conventions correctly.The card and dashboard CRUD operations are well-implemented:
- Correct HTTP methods for each operation
- Consistent parameter handling
runCardQueryappropriately uses POST for query executionThe same input validation suggestions from the previous comment apply to the ID parameters in these methods.
68-86:✅ Verification successful
Verify the authentication token field name.
The utility methods are well-structured for Metabase API communication. However, verify that
this.$auth.oauth_access_tokenis the correct field name for the Metabase session token, as this typically contains a session ID rather than an OAuth token.
🏁 Script executed:
#!/bin/bash # Description: Check if other Metabase components use the same auth field name # Expected: Consistent usage of oauth_access_token across Metabase components rg -A 3 "oauth_access_token.*metabase\|metabase.*oauth_access_token" --type=js --type=mjsLength of output: 119
🏁 Script executed:
#!/bin/bash # Search for occurrences of oauth_access_token in .js and .mjs files rg 'oauth_access_token' -n --glob '*.js' --glob '*.mjs'Length of output: 51476
Auth token property is correct and consistent.
We’ve confirmed that
this.$auth.oauth_access_tokenis the standard token field used across all components (including Metabase) for API calls. No changes needed.
components/metabase/actions/create-dashboard/create-dashboard.mjs
Outdated
Show resolved
Hide resolved
jcortes
force-pushed
the
metabase-new-components
branch
from
f53e989 to
6e084aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/metabase/common/constants.mjs (1)
1-17: Add missing visualization types to maintain API compatibility.The VISUALIZATION_TYPES array is missing several supported types from the current Metabase API. As identified in the previous review, the following types should be added: detail, pivot, histogram, sankey, donut, and sunburst.
🧹 Nitpick comments (2)
components/metabase/common/constants.mjs (2)
34-46: Consider validating input parameters in formatting functions.The formatting functions assume valid input objects. Consider adding basic validation to prevent runtime errors if null/undefined objects are passed.
export const formatCardSummary = (card) => { + if (!card) return "Invalid card"; const { id, name, display, } = card; return `Card "${name}" (ID: ${id}, Type: ${display})`; }; export const formatDashboardSummary = (dashboard) => { + if (!dashboard) return "Invalid dashboard"; const { id, name, dashcards = [], } = dashboard; return `Dashboard "${name}" (ID: ${id}, Cards: ${dashcards.length})`; };
47-47: Remove stray line number.There appears to be a formatting artifact with a bare line number that should be removed.
-47
📜 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 (11)
components/metabase/.gitignore(0 hunks)components/metabase/README.md(1 hunks)components/metabase/actions/create-card/create-card.mjs(1 hunks)components/metabase/actions/create-dashboard/create-dashboard.mjs(1 hunks)components/metabase/actions/get-dashboard/get-dashboard.mjs(1 hunks)components/metabase/actions/get-database/get-database.mjs(1 hunks)components/metabase/actions/run-query/run-query.mjs(1 hunks)components/metabase/common/constants.mjs(1 hunks)components/metabase/common/utils.mjs(1 hunks)components/metabase/metabase.app.mjs(1 hunks)components/metabase/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- components/metabase/.gitignore
🚧 Files skipped from review as they are similar to previous changes (5)
- components/metabase/README.md
- components/metabase/package.json
- components/metabase/actions/create-card/create-card.mjs
- components/metabase/actions/get-dashboard/get-dashboard.mjs
- components/metabase/actions/create-dashboard/create-dashboard.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
components/metabase/common/utils.mjs (1)
40-46: LGTM: parseArray function is correctly implemented.The function properly handles non-array inputs and applies JSON parsing to each array element.
components/metabase/actions/run-query/run-query.mjs (1)
1-43: LGTM: Well-structured action implementation.The action correctly:
- Uses the app component for API calls
- Maps camelCase props to snake_case API parameters
- Provides appropriate documentation links
- Exports a summary message and returns the response
components/metabase/actions/get-database/get-database.mjs (1)
1-63: LGTM: Consistent and well-implemented action.The action follows the established pattern and correctly:
- Maps all camelCase properties to snake_case API parameters
- Provides clear option labels and descriptions
- Uses proper parameter validation through string options
- Maintains consistency with other actions in the integration
components/metabase/metabase.app.mjs (3)
6-57: LGTM: Well-designed prop definitions with good defensive programming.The async options for dynamic loading are well-implemented, and the defensive handling in the collection options (line 29) that provides "Root Collection" as a fallback for falsy collection names is excellent practice.
59-84: LGTM: Proper authentication and request handling.The URL construction, header management, and generic request method are correctly implemented. The x-api-key authentication approach is appropriate for Metabase.
85-159: LGTM: Comprehensive API method coverage.The API methods provide good coverage of Metabase functionality and follow consistent patterns. The parameter destructuring and path construction are well-implemented.
components/metabase/common/constants.mjs (1)
24-32: Well-implemented error parsing with proper fallback hierarchy.The error parsing function correctly handles the nested response structure from Metabase API responses and provides appropriate fallbacks. The logic prioritizes specific error messages and gracefully degrades to generic messages.
jcortes
force-pushed
the
metabase-new-components
branch
from
6e084aa to
8d86dd6
Compare
jcortes
force-pushed
the
metabase-new-components
branch
from
8d86dd6 to
2be9dd1
Compare
jcortes
force-pushed
the
metabase-new-components
branch
from
2be9dd1 to
9296cd5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (9)
components/metabase/.gitignore(0 hunks)components/metabase/README.md(1 hunks)components/metabase/actions/create-dashboard/create-dashboard.mjs(1 hunks)components/metabase/actions/get-dashboard/get-dashboard.mjs(1 hunks)components/metabase/actions/get-database/get-database.mjs(1 hunks)components/metabase/actions/run-query/run-query.mjs(1 hunks)components/metabase/common/utils.mjs(1 hunks)components/metabase/metabase.app.mjs(1 hunks)components/metabase/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- components/metabase/.gitignore
🚧 Files skipped from review as they are similar to previous changes (7)
- components/metabase/package.json
- components/metabase/README.md
- components/metabase/common/utils.mjs
- components/metabase/actions/run-query/run-query.mjs
- components/metabase/actions/get-dashboard/get-dashboard.mjs
- components/metabase/actions/get-database/get-database.mjs
- components/metabase/actions/create-dashboard/create-dashboard.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (3)
components/metabase/metabase.app.mjs (3)
71-88: LGTM! Well-structured request handling methods.The URL construction and generic request wrapper follow good patterns. The separation of concerns between
getUrl,getHeaders, and_makeRequestprovides good modularity and reusability.
89-149: LGTM! API methods are well-implemented.The HTTP verb methods and individual API endpoint methods follow consistent patterns and correctly utilize the generic
_makeRequestmethod. The parameter destructuring and path construction are handled appropriately.
7-68:⚠️ Potential issueFix type inconsistency across ID property definitions.
The property definitions have inconsistent types for ID fields:
databaseIdandcardIdusetype: "integer"collectionIdanddashboardIdusetype: "string"This inconsistency could cause confusion and potential type-related issues.
Apply this diff to standardize the types:
collectionId: { - type: "string", + type: "integer", label: "Collection ID", description: "The ID of the collection",dashboardId: { - type: "string", + type: "integer", label: "Dashboard ID", description: "The ID of the dashboard",Likely an incorrect or invalid review comment.
WHY
Resolves #16923
Summary by CodeRabbit
New Features
Documentation
Chores
Refactor