-
Couldn't load subscription status.
- Fork 5.5k
New Components - contentstack #15037
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces a comprehensive Contentstack integration for Pipedream, featuring actions for creating, updating, and publishing entries, as well as sources for tracking new assets, entries, and published entries. The implementation includes utility functions, API interaction methods, and webhook management, providing a robust framework for interacting with the Contentstack content management system. Changes
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
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: 13
🧹 Nitpick comments (16)
components/contentstack/actions/common/entries.mjs (3)
20-37: Graceful fallback for content type errors.
When an error occurs in building field props, the code gracefully returns a JSON-based fallback. This is a good idea to avoid blocking the user. However, consider logging the error for easier debugging.} catch (err) { + console.error("Error building field props:", err); return { entryObj: {
53-76: Efficient reference and file fetch.
ThegetOptions()method efficiently retrieves entries and assets for reference/file fields. Be mindful of potential performance implications if there are many references or large files. Consider adding pagination or caching in the future.
77-94: Dynamic field prop building.
Dynamic property construction is well-organized. The code loops through each field in the schema and configures the correct prop. Make sure to handle deeply nested fields (e.g., groups within groups) if your use case requires it in the future.components/contentstack/contentstack.app.mjs (5)
8-21: Branch IDs property looks good.
The asynchronous options approach is correct, returning an array of branch UIDs. For enhanced usability, consider labeling the returned items (like "main" or other branch names) if available.
52-60: Publishing environment selection
The code retrieves and returns environment names. This is helpful but be mindful of name / UID mismatch. If needed, you may want to store both environment UID and environment name in the future.
97-112: Webhook management
The creation and deletion of webhooks are straightforward. Consider adding rate limiting or retries for potential network issues.
113-132: Branch, content types, and entries listing
Listing endpoints are distinct and consistent. If these are frequently called, consider caching or a more efficient approach (especially with large datasets).
145-160: Entry retrieval
The ability to fetch a single entry or content type is crucial for read operations. Keep in mind: if an entry is not found, handle potential 404 errors gracefully.components/contentstack/sources/publish-entry-instant/test-event.mjs (2)
88-409: Comprehensive content type schema
The schema is thorough. Great job providing all fields and structure. If these are for test events only, confirm that no real user data or internal secrets are exposed.
410-438: Environment details
Including environment details is useful. Just be cautious if environment-level secrets or private URLs are present.Would you like help generating a sanitized version of this test event?
🧰 Tools
🪛 Gitleaks (8.21.2)
431-431: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
components/contentstack/sources/new-entry-instant/test-event.mjs (1)
73-487: Comprehensive content type schema
The schema includes all relevant fields. As this is a static sample, consider removing confidential or environment-specific data if not needed for the test.components/contentstack/sources/new-entry-instant/new-entry-instant.mjs (1)
1-29: Consider extracting common metadata generation logic.Both instant sources share similar metadata generation logic. Consider creating a utility function in the common base module.
Example implementation for base.mjs:
export const generateMetadata = (event, type, dataPath) => { const id = dataPath.split('.').reduce((obj, key) => obj?.[key], event); if (!id) { throw new Error('Received invalid event structure'); } return { id, summary: `New ${type} created with UID: ${id}`, ts: Date.parse(event.triggered_at), }; };Then in the sources:
generateMeta(event) { return generateMetadata(event, 'entry', 'data.entry.uid'); }components/contentstack/sources/publish-entry-instant/publish-entry-instant.mjs (1)
19-26: Consider enhancing the event metadata.While the current metadata is functional, consider including additional useful information from the event data such as:
- Content type
- Entry title/name
- Publishing environment
generateMeta(event) { const id = event.data.entry.uid; return { id, - summary: `New entry published with ID: ${id}`, + summary: `New entry "${event.data.entry.title}" published to ${event.data.environment} [${id}]`, ts: Date.parse(event.triggered_at), + contentType: event.data.content_type, }; }components/contentstack/actions/update-entry/update-entry.mjs (1)
29-34: Consider caching isUpdate resultThe
isUpdate()method is called during entry building. Consider caching its result if called multiple times during the same operation.methods: { ...common.methods, + _isUpdate: true, isUpdate() { - return true; + return this._isUpdate; }, },components/contentstack/sources/common/base.mjs (1)
61-66: Improve abstract method implementationThe abstract methods should be more descriptive about their requirements.
Consider updating the implementation:
getChannels() { - throw new Error("getChannels is not implemented"); + throw new Error( + "getChannels() must be implemented by child classes to return an array of channel names" + ); }, generateMeta() { - throw new Error("generateMeta is not implemented"); + throw new Error( + "generateMeta(body) must be implemented by child classes to return event metadata: { id, summary, ts }" + ); },components/contentstack/actions/publish-entry/publish-entry.mjs (1)
70-71: Consider enhancing the success message with more details.The success message could include more useful information like the content type and scheduled time (if applicable).
- $.export("$summary", `Successfully published entry with UID ${this.entryId}`); + $.export("$summary", `Successfully published ${this.contentType} entry (UID: ${this.entryId})${this.scheduledAt ? ` scheduled for ${this.scheduledAt}` : ''}`);
📜 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 (14)
components/contentstack/actions/common/entries.mjs(1 hunks)components/contentstack/actions/create-entry/create-entry.mjs(1 hunks)components/contentstack/actions/publish-entry/publish-entry.mjs(1 hunks)components/contentstack/actions/update-entry/update-entry.mjs(1 hunks)components/contentstack/common/utils.mjs(1 hunks)components/contentstack/contentstack.app.mjs(1 hunks)components/contentstack/package.json(2 hunks)components/contentstack/sources/common/base.mjs(1 hunks)components/contentstack/sources/new-asset-instant/new-asset-instant.mjs(1 hunks)components/contentstack/sources/new-asset-instant/test-event.mjs(1 hunks)components/contentstack/sources/new-entry-instant/new-entry-instant.mjs(1 hunks)components/contentstack/sources/new-entry-instant/test-event.mjs(1 hunks)components/contentstack/sources/publish-entry-instant/publish-entry-instant.mjs(1 hunks)components/contentstack/sources/publish-entry-instant/test-event.mjs(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.21.2)
components/contentstack/sources/new-asset-instant/test-event.mjs
3-3: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
components/contentstack/sources/new-entry-instant/test-event.mjs
3-3: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
components/contentstack/sources/publish-entry-instant/test-event.mjs
3-3: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
431-431: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
440-440: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (25)
components/contentstack/actions/common/entries.mjs (3)
9-19: Props definition is comprehensive.
Good job including both the required props (contentstack and contentType) and leaving the door open for additional props. The structure ensures that users can configure dynamic properties based on the chosen content type.
95-125: Robust entry object construction.
The buildEntry method properly translates form fields into an entry object. The handle for reference fields—checking field.multiple—is well done. For large arrays, consider the potential need for chunking or batch processing.
38-52: Clear type mapping logic.
The getType function is straightforward, covering the primary data types. However, for future-proofing, consider enumerating any unhandled data types or verifying them through robust schema constraints.
✅ Verification successful
Based on the search results, I can see that in the Contentstack component, there are several additional data types being used beyond those handled in the current getType function:
- "text" - Currently mapped to "string"
- "file" - Currently mapped to "string"
- "taxonomy" - Currently mapped to "string"
- "group" - Currently mapped to "string"
- "global_field" - Currently mapped to "string"
- "number" - Currently mapped to "string"
- "reference" - Has special handling in other methods
Let me verify if there's any specific type mapping logic defined elsewhere in the codebase:
Let me try a different search pattern to look for any type-related configurations or constants:
Based on the comprehensive analysis of the codebase, I can now provide a final response:
Type mapping is appropriately implemented for current usage
The current implementation in getType correctly handles the essential data types (boolean, json) with specific mappings, while defaulting others to either string or string[] based on the multiple flag. This is sufficient because:
- The special cases (
boolean,json) are explicitly handled where different behavior is needed - Other types (text, file, taxonomy, etc.) are appropriately defaulted to string types since they represent textual or path-based data
- Complex types like
referenceandfilehave separate handling in thegetOptionsmethod where their specific behaviors are implemented
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for other data_type usage to verify coverage:
rg --no-heading --line-number "data_type" -A 5
Length of output: 34110
Script:
#!/bin/bash
# Search for type mapping constants or configurations
rg -g '!*.{md,txt}' "type.*map" -A 3 components/contentstack/
Length of output: 63
Script:
#!/bin/bash
# Look for type-related constants or configurations
rg -g '!*.{md,txt}' "const.*type" -i components/contentstack/
# Also search for any schema or type definitions
ast-grep --pattern 'const $_ = {
$$$
type: $$$
$$$
}' components/contentstack/
Length of output: 655
components/contentstack/contentstack.app.mjs (7)
36-51: Entry ID property
The method uses the selected content type to list entries. This is useful for referencing the correct item to update or publish. Keep an eye on performance if content type entries become large.
61-74: Locale property
Fetching locales is consistent with other property definitions. The usage of label/value pairs is clear. If you have a large number of locales, consider grouping or searching for user convenience.
82-95: _makeRequest centralizes requests
This is a great pattern to consolidate request logic. Ensure you sanitize or encode special parameters to avoid injection or malformed queries.
133-144: Environments and locales listing
This keeps the logic symmetrical with listing other resources. Be sure to handle edge cases, e.g., if no environments or locales are found.
161-187: Creating, updating, and publishing entries
Each method is consistent with _makeRequest, simplifying the logic. If concurrency might arise (e.g., parallel updates to the same entry), ensure robust handling of potential conflicts.
188-198: Listing and getting assets
Good addition for retrieving assets. The getAsset() method references this.stackId and this.assetId, which might not always be set. Confirm these are indeed defined or flagged if absent.
77-81: Base URL generation
Clever logic adjusting the domain based on region. Double-check the domain concatenation for potential trailing slash or subdomain mismatches.
✅ Verification successful
Let me analyze the usage of _baseUrl() in the codebase to verify the concern about domain concatenation.
Base URL concatenation is correctly implemented
The implementation of _baseUrl() in contentstack.app.mjs is well-structured and safe:
- The region value from auth is used as the base (
this.$auth.region) - The domain is correctly concatenated with the appropriate TLD based on the region:
- Uses "io" when region is "https://"
- Uses "com" for other regions
- The "/v3" version path is properly appended at the end
- The method is consistently used throughout the app via
_makeRequest()
The original concern about potential trailing slash or subdomain mismatches is unfounded as the implementation handles the URL construction properly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for usage of _baseUrl() to confirm correct usage:
rg "_baseUrl"
Length of output: 65634
Script:
#!/bin/bash
# Let's examine the actual implementation of _baseUrl() in contentstack.app.mjs
cat components/contentstack/contentstack.app.mjs
Length of output: 4993
components/contentstack/sources/publish-entry-instant/test-event.mjs (3)
7-86: Detailed event payload
This mock event is comprehensive. Excellent for testing "publish entry" scenarios. Just ensure no sensitive data is inadvertently included in logs.
439-449: Branch data
The branch metadata is helpful for debugging. Again, ensure that it does not contain private info.
🧰 Tools
🪛 Gitleaks (8.21.2)
440-440: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
451-451: Triggered timestamp
Including the timestamp is good for auditing. No issues found.
components/contentstack/sources/new-entry-instant/test-event.mjs (3)
5-72: Thorough mock entry data
The test event data is very detailed. This is great for validating how well your "new entry" logic works. Be mindful that large payloads can cause performance issues in certain test scenarios.
488-492: Branch config
Providing the branch UID is helpful to ensure the correct environment is selected. Just double-check that you are indeed referencing main in your real workflow or if it should be dynamic.
494-496: Event type and timestamp
Clear definitions of the event type and triggered time. No issues, but ensure you handle time zone or locale differences in any downstream processes.
components/contentstack/package.json (1)
Line range hint 3-18: LGTM! Verify platform version compatibility.
The version bump and dependency addition look good. The semver range for @pipedream/platform allows minor updates which is appropriate.
Let's verify the latest compatible version:
✅ Verification successful
Platform version ^3.0.3 is compatible with the codebase
The version range ^3.0.3 specified in the package.json is consistent with other components in the codebase. Multiple components are already using version 3.x.x of @pipedream/platform, with ^3.0.3 being the latest version used. This indicates proper version alignment and compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest @pipedream/platform version used across the codebase
fd package.json | xargs cat | jq -r 'select(.dependencies."@pipedream/platform") | .dependencies."@pipedream/platform"' | sort -u
Length of output: 334
components/contentstack/sources/publish-entry-instant/publish-entry-instant.mjs (2)
6-11: LGTM! Component configuration is well-structured.
The component metadata is properly defined with a clear name, description, and appropriate type/dedupe settings.
14-18: Verify the webhook channel name.
The channel name content_types.entries.publish needs verification to ensure it matches Contentstack's webhook event naming convention.
✅ Verification successful
Based on the code inspection, I can now provide a definitive response about the webhook channel name verification:
The webhook channel name is correct and matches Contentstack's event naming convention.
The code shows multiple source components in the Contentstack integration that use consistent webhook channel naming patterns:
content_types.entries.publishfor publish eventscontent_types.entries.createfor creation eventsassets.createfor asset creation events
The channel name follows Contentstack's hierarchical event naming pattern of resource.subresource.action. This is a valid webhook channel name for listening to entry publish events.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of this webhook channel name to verify consistency
rg -l "content_types.entries.publish" --type js
Length of output: 47
Script:
#!/bin/bash
# Let's check the Contentstack documentation or configuration files
rg -i "contentstack.*webhook" --type js -A 5 -B 5
# Also search for any webhook event patterns
rg -i "webhook.*events?" --type js -A 5 -B 5
# Look for similar event patterns
rg "entries\.(publish|unpublish|delete)" --type js
Length of output: 151
Script:
#!/bin/bash
# Let's check the README or documentation files for Contentstack integration
fd -e md -e txt -x cat {} | rg -i "contentstack|webhook" -C 5
# Check the specific component's documentation
cat components/contentstack/sources/publish-entry-instant/README.md
# Look for any test files that might contain webhook event patterns
fd -e test.js -e test.mjs -e spec.js -e spec.mjs -x cat {} | rg -i "contentstack.*webhook|webhook.*event"
Length of output: 72455
Script:
#!/bin/bash
# Let's check the app file for any configuration or documentation
cat components/contentstack/app/contentstack.app.mjs
# Look for any test files specifically in the contentstack directory
fd -e test.js -e test.mjs -e spec.js -e spec.mjs . components/contentstack/ -x cat {}
# Check for any other files in the contentstack component that might contain webhook documentation
fd . components/contentstack/ -x cat {}
Length of output: 46641
components/contentstack/sources/new-asset-instant/test-event.mjs (1)
5-21: LGTM! Comprehensive asset data structure.
The test event includes all necessary fields for asset creation events, providing good coverage for testing webhook handlers.
components/contentstack/common/utils.mjs (2)
1-13: LGTM! Clean implementation of array parsing.
The parseArray function handles edge cases well and provides clear error messages.
15-41: Add error handling for malformed JSON keys.
Consider adding validation for the structure of parsed JSON to ensure it matches expected schemas.
components/contentstack/actions/create-entry/create-entry.mjs (1)
8-8: Consider updating version number
The version is set to "0.0.1" which indicates this is the first release. Ensure this aligns with your versioning strategy.
✅ Verification successful
Version number is consistent with other Contentstack components
The version "0.0.1" is consistent across all Contentstack components in the codebase, indicating this is part of a deliberate versioning strategy for the entire Contentstack integration. No change is needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if this is truly the first version of this component
rg -g '!node_modules' -l 'contentstack-create-entry'
Length of output: 114
Script:
#!/bin/bash
# Check git history for this file to see if this is truly the first version
git log --oneline -- components/contentstack/actions/create-entry/create-entry.mjs
# Look for any package.json in the component directory that might have version info
fd package.json --exec cat {} \; --search-path components/contentstack/actions/create-entry/
# Search for other version numbers in related contentstack files
rg -g '!node_modules' 'version:.*' components/contentstack/
Length of output: 788
components/contentstack/actions/publish-entry/publish-entry.mjs (2)
1-9: LGTM! Module definition and imports are well structured.
The module metadata is properly defined with appropriate versioning and documentation links.
33-41: Verify the type consistency for the locales prop.
There appears to be a potential type mismatch:
- The propDefinition references "locale" (singular)
- The type is defined as "string[]" (array)
This inconsistency could lead to runtime issues if the propDefinition returns a single value instead of an array.
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.
LGTM!
Resolves #14838.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores