New Components - printful #15436
New Components - printful #15436
Conversation
luancazarine
added
the
ai-assisted
WalkthroughThis pull request introduces a comprehensive set of components for the Printful integration, including actions for calculating shipping rates, creating orders, updating products, and webhook sources for tracking new orders, product additions, and order status updates. The implementation provides a structured approach to interacting with the Printful API, offering methods to manage store operations, retrieve shipping rates, create and update products, and handle real-time event notifications. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
Sources - New Order (Instant) - New Product Added (Instant) - New Order Status Updated (Instant) Actions - Create Order - Calculate Shipping Rates - Update Product
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (11)
components/printful/printful.app.mjs (2)
1-3: Consider centralizing constants and clarifying dependencies.
All imports look fine. To improve maintainability, consider centralizing shared constants (likeLIMIT) and ensuring version-locked dependencies to reduce potential API-breaking changes.
64-74: Add retry and error handling logic.
_makeRequestis well-structured, but you may want to handle transient network errors or implement a retry pattern for resiliency.components/printful/common/utils.mjs (1)
4-24: Consider stricter error reporting or built-in JSON reviver.
While catching parse errors is good for flexibility, you might benefit from more explicit error logging or adding a JSON reviver for custom transformations if needed.components/printful/sources/order-status-updated-instant/order-status-updated-instant.mjs (1)
14-24: Consider enhancing the summary format.The summary format could be more user-friendly by using a more natural language structure.
- return `Order ${body.data.order.id} status updated to ${body.data.order.status}`; + return `Order #${body.data.order.id} status has been updated to "${body.data.order.status}"`;components/printful/actions/create-order/create-order.mjs (2)
6-6: Consider bumping version to 1.0.0.Since this is a new component being released to production, it's recommended to start with version 1.0.0 instead of 0.0.1 to indicate a stable, production-ready release.
104-107: Add input validation for countryCode.The
additionalPropsmethod should validate the countryCode input to ensure it's a valid country code before using it in the comparison.additionalProps(props) { + const validCountryCodes = ["BR", "US", "CA", /* add other valid codes */]; + if (!validCountryCodes.includes(this.countryCode)) { + throw new Error(`Invalid country code: ${this.countryCode}`); + } props.taxNumber.optional = this.countryCode != "BR"; return {}; },components/printful/sources/new-order-instant/test-event.mjs (1)
1-217: Improve test event data quality.The test event data could be improved in several ways:
- Use realistic values instead of placeholders (e.g., "string" for address2)
- Include edge cases (e.g., orders with multiple items)
- Add comments explaining the purpose of each section
Additionally, consider extracting this test data into a shared fixture that can be reused across different test files to maintain consistency.
components/printful/sources/new-product-added-instant/test-event.mjs (2)
3-3: Update the created timestampThe timestamp (1622456737) corresponds to May 31, 2021. Consider using a more recent timestamp for the sample event.
- "created": 1622456737, + "created": 1705363200, // January 16, 2024
1-17: Consider enhancing the test event dataA few suggestions to make the test event more robust:
- Add more realistic product details (e.g., SKU, prices, categories)
- Include validation-related fields that might be required
- Use a more descriptive external_id format
Would you like me to provide an enhanced version of the test event with more comprehensive data?
components/printful/sources/new-product-added-instant/new-product-added-instant.mjs (2)
9-9: Consider starting with version 1.0.0For initial releases, it's generally recommended to start with version 1.0.0 to indicate a stable, production-ready component.
- version: "0.0.1", + version: "1.0.0",
14-18: Simplify getEventType implementationSince you're returning an array with a single string, consider simplifying the implementation.
- getEventType() { - return [ - "product_synced", - ]; - }, + getEventType: () => ["product_synced"],
📜 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/printful/actions/calculate-shipping-rates/calculate-shipping-rates.mjs(1 hunks)components/printful/actions/create-order/create-order.mjs(1 hunks)components/printful/actions/update-product/update-product.mjs(1 hunks)components/printful/common/constants.mjs(1 hunks)components/printful/common/utils.mjs(1 hunks)components/printful/package.json(1 hunks)components/printful/printful.app.mjs(1 hunks)components/printful/sources/common/base.mjs(1 hunks)components/printful/sources/new-order-instant/new-order-instant.mjs(1 hunks)components/printful/sources/new-order-instant/test-event.mjs(1 hunks)components/printful/sources/new-product-added-instant/new-product-added-instant.mjs(1 hunks)components/printful/sources/new-product-added-instant/test-event.mjs(1 hunks)components/printful/sources/order-status-updated-instant/order-status-updated-instant.mjs(1 hunks)components/printful/sources/order-status-updated-instant/test-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- components/printful/common/constants.mjs
- components/printful/package.json
🧰 Additional context used
🪛 GitHub Check: Lint Code Base
components/printful/actions/update-product/update-product.mjs
[warning] 27-27:
Component prop alert must have a label. See https://pipedream.com/docs/components/guidelines/#props
[warning] 27-27:
Component prop alert must have a description. See https://pipedream.com/docs/components/guidelines/#props
components/printful/sources/common/base.mjs
[warning] 11-11:
Component prop alert must have a label. See https://pipedream.com/docs/components/guidelines/#props
[warning] 11-11:
Component prop alert must have a description. See https://pipedream.com/docs/components/guidelines/#props
🔇 Additional comments (7)
components/printful/printful.app.mjs (3)
7-27: Ensure store ID is validated before use.
The newstoreIdprop definition appears correct, but you might want to validate or sanitizestoreIdprior to using it in downstream methods.
28-52: Confirm product ID usage and validation.
Similar tostoreId, ensureproductIdis validated or sanitized to avoid issues with special characters or potential injection concerns if user input is untrusted.
75-122: Confirm webhook deletion workflow.
deleteWebhookcalls"DELETE"on the generic/webhookspath. If Printful’s API requires a specific webhook ID to delete an existing webhook, confirm that you’re passing it correctly, or consider exposing a webhook ID prop.components/printful/common/utils.mjs (1)
1-3: Early return logic is clear.
Good job returningundefinedwhenobjis falsy, preventing unnecessary parsing attempts.components/printful/sources/new-order-instant/new-order-instant.mjs (1)
1-3: Imports follow expected structure.
The import from../common/base.mjsand./test-event.mjsis correctly organized, ensuring code reuse and proper test data integration.components/printful/sources/order-status-updated-instant/order-status-updated-instant.mjs (1)
4-12: LGTM! Well-structured component definition.The component is well-defined with appropriate key, name, description, and deduplication strategy. The version number correctly starts at 0.0.1 for a new component.
components/printful/sources/new-product-added-instant/new-product-added-instant.mjs (1)
4-27: Verify webhook configuration with Printful APIThe component implements a webhook for new product events. Let's verify the webhook configuration matches Printful's API requirements.
✅ Verification successful
Webhook configuration is properly implemented and valid
The component correctly implements Printful's webhook system for product sync events:
- Uses valid "product_synced" event type
- Properly handles webhook registration and cleanup
- Correctly processes sync_product data from webhook payload
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Printful webhook documentation or examples in the codebase rg -g '!node_modules' -A 5 'printful.*webhook' .Length of output: 50
Script:
#!/bin/bash # Search for common.mjs and related files fd -e mjs common fd -e mjs printful # Look for API documentation references and webhook configuration rg -g '!node_modules' -A 10 'printful.*api' . rg -g '!node_modules' -A 10 'webhook.*event' . # Search for event type definitions ast-grep --pattern 'getEventType() { $$$'Length of output: 66657
Script:
#!/bin/bash # Check printful_oauth common files and webhook configuration cat components/printful_oauth/sources/common/common.mjs # Look for webhook-related code in printful app files cat components/printful/printful.app.mjs cat components/printful_oauth/printful_oauth.app.mjsLength of output: 4942
Script:
#!/bin/bash # Look for Printful event type definitions or constants fd -e mjs -e js printful | xargs rg -l "product.*sync" # Search for any documentation or event type definitions rg -g '!node_modules' -A 5 'product.*sync|webhook.*type' components/printful*Length of output: 3377
components/printful/sources/new-order-instant/new-order-instant.mjs
Outdated
Show resolved
Hide resolved
components/printful/sources/order-status-updated-instant/test-event.mjs
Outdated
Show resolved
Hide resolved
components/printful/sources/new-product-added-instant/test-event.mjs
Outdated
Show resolved
Hide resolved
components/printful/sources/new-product-added-instant/new-product-added-instant.mjs
Show resolved
Hide resolved
components/printful/actions/calculate-shipping-rates/calculate-shipping-rates.mjs
Outdated
Show resolved
Hide resolved
components/printful/sources/new-product-added-instant/test-event.mjs
Outdated
Show resolved
Hide resolved
components/printful/sources/order-status-updated-instant/order-status-updated-instant.mjs
Outdated
Show resolved
Hide resolved
components/printful/sources/new-order-instant/new-order-instant.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
components/printful/sources/common/base.mjs (1)
43-51:⚠️ Potential issueAdd error handling for malformed webhook payloads.
The run method should handle cases where the expected fields are missing in the webhook payload.
async run({ body }) { + if (!body?.data) { + throw new Error('Webhook payload is missing required data field'); + } const modelField = this.getModelField(); + if (!body.data[modelField]) { + throw new Error(`Webhook payload is missing required ${modelField} field`); + } const ts = body.created; this.$emit(body, { id: `${body.data[modelField].id}-${ts}`, summary: this.getSummary(body), ts, }); },components/printful/actions/calculate-shipping-rates/calculate-shipping-rates.mjs (1)
69-91: 🛠️ Refactor suggestionEnhance error handling and success message.
The run method could benefit from better validation and a more informative success message.
async run({ $ }) { + const items = parseObject(this.items); + if (!items?.length) { + throw new Error('At least one item is required to calculate shipping rates'); + } + for (const item of items) { + if (!item.variant_id || !item.quantity) { + throw new Error('Each item must have a variant_id and quantity'); + } + } + const shippingRates = await this.printful.fetchShippingRates({ $, headers: { "X-PF-Store-Id": this.storeId, }, data: { recipient: { address1: this.address1, city: this.city, state_code: this.stateCode, country_code: this.countryCode, zip: this.zip, phone: this.phone, }, - items: parseObject(this.items), + items, currency: this.currency, locale: this.locale, }, }); - $.export("$summary", "Fetched shipping rates successfully"); + $.export("$summary", `Found ${shippingRates.length} shipping options for ${this.city}, ${this.stateCode}`); return shippingRates; },
🧹 Nitpick comments (9)
components/printful/sources/order-status-updated-instant/order-status-updated-instant.mjs (1)
22-24: Add null checks to prevent potential runtime errors.The
getSummarymethod should handle cases where the body structure might be incomplete.Consider adding null checks:
- getSummary(body) { - return `Order ${body.data.order.id} status updated to ${body.data.order.status}`; + getSummary(body) { + const orderId = body?.data?.order?.id ?? 'unknown'; + const status = body?.data?.order?.status ?? 'unknown'; + return `Order ${orderId} status updated to ${status}`;components/printful/common/constants.mjs (1)
1-11: Add JSDoc comments to document the constants.Consider adding JSDoc comments to explain the purpose and usage of these constants:
+/** + * Maximum number of items to return per page in API requests + * @constant {number} + */ export const LIMIT = 100; +/** + * Supported locale options for Printful API responses + * @constant {Array<{label: string, value: string}>} + */ export const LOCALE_OPTIONS = [ { label: "English (US)", value: "en_US", }, { label: "Spanish (Spain)", value: "es_ES", }, ];components/printful/actions/calculate-shipping-rates/calculate-shipping-rates.mjs (1)
50-54: Improve items prop description and validation.The items prop could benefit from better documentation and type validation.
items: { type: "string[]", label: "Items", - description: "A list of items in JSON format. **Example: [{\"variant_id\": \"123456\", \"external_variant_id\": \"123456\", \"quantity\": 123, \"value\": \"12345\" }]**", + description: "A list of items to calculate shipping for. Each item must include `variant_id` and `quantity`. Optional fields: `external_variant_id` and `value`.", + options: [ + { + label: "Example", + value: JSON.stringify([{ + variant_id: "123456", + quantity: 1, + external_variant_id: "123456", + value: "12345" + }]) + } + ] },components/printful/printful.app.mjs (2)
11-11: Fix typo in storeId description.There's a typo in the description: "provide s store" should be "provide a store".
- description: "Select a store or provide s store ID to use for the request", + description: "Select a store or provide a store ID to use for the request",
92-98: Use consistent HTTP method casing.HTTP methods should be consistently lowercase throughout the codebase.
fetchShippingRates(opts = {}) { return this._makeRequest({ - method: "POST", + method: "post", path: "/shipping/rates", ...opts, }); },components/printful/sources/new-order-instant/test-event.mjs (4)
1-5: Consider using dynamic timestamps and realistic store IDs in test events.The hardcoded timestamp and small store ID might not represent real-world scenarios effectively.
export default { "type": "order_created", - "created": 1622456737, + "created": Math.floor(Date.now() / 1000), - "store": 12, + "store": 123456, "retries": 2,
87-142: Reduce duplication in test event structure.The branding items section is an exact duplicate of the order items section. Consider:
- Creating a shared constant for the item structure
- Using it in both sections with minimal differences
Consider moving this to a separate file:
// test-event-constants.mjs export const SAMPLE_ITEM = { // ... common item structure };
182-202: Use valid tracking number format in test events.The current tracking number (0) and URL won't represent real tracking scenarios effectively.
"id": 10, "carrier": "FEDEX", "service": "FedEx SmartPost", - "tracking_number": 0, + "tracking_number": "1234567890", - "tracking_url": "https://www.fedex.com/fedextrack/?tracknumbers=0000000000", + "tracking_url": "https://www.fedex.com/fedextrack/?tracknumbers=1234567890",
203-214: Use consistent test data in packing slip.
- Use the same test email/phone format as in recipient section
- Use a working example URL for the logo
"packing_slip": { - "email": "your-name@your-domain.com", + "email": "test@example.com", - "phone": "+371 28888888", + "phone": "555-0123", "message": "Message on packing slip", - "logo_url": "http://www.your-domain.com/packing-logo.png", + "logo_url": "https://example.com/logo.png",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
components/printful/actions/calculate-shipping-rates/calculate-shipping-rates.mjs(1 hunks)components/printful/actions/create-order/create-order.mjs(1 hunks)components/printful/common/constants.mjs(1 hunks)components/printful/printful.app.mjs(1 hunks)components/printful/sources/common/base.mjs(1 hunks)components/printful/sources/new-order-instant/new-order-instant.mjs(1 hunks)components/printful/sources/new-order-instant/test-event.mjs(1 hunks)components/printful/sources/new-product-added-instant/test-event.mjs(1 hunks)components/printful/sources/order-status-updated-instant/order-status-updated-instant.mjs(1 hunks)components/printful/sources/order-status-updated-instant/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- components/printful/sources/new-product-added-instant/test-event.mjs
- components/printful/sources/order-status-updated-instant/test-event.mjs
- components/printful/actions/create-order/create-order.mjs
- components/printful/sources/new-order-instant/new-order-instant.mjs
🧰 Additional context used
📓 Learnings (1)
components/printful/printful.app.mjs (1)
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15436
File: components/printful/printful.app.mjs:55-63
Timestamp: 2025-01-29T22:59:38.825Z
Learning: Console.log statements should be removed before merging PRs to maintain code quality and prevent potential security risks from exposing sensitive information in logs.
🪛 GitHub Check: Lint Code Base
components/printful/sources/common/base.mjs
[warning] 11-11:
Component prop alert must have a label. See https://pipedream.com/docs/components/guidelines/#props
[warning] 11-11:
Component prop alert must have a description. See https://pipedream.com/docs/components/guidelines/#props
components/printful/sources/order-status-updated-instant/order-status-updated-instant.mjs
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (7)
components/printful/sources/order-status-updated-instant/order-status-updated-instant.mjs (3)
1-2: LGTM! Clean imports with good separation of concerns.The imports are well-structured, utilizing the common base functionality and separate test events.
4-11: Naming and configuration look good!The component configuration follows best practices and the naming appropriately avoids the 'new' prefix for an 'updated' event source, as previously discussed in the reviews.
🧰 Tools
🪛 GitHub Check: Lint Code Base
[warning] 7-7:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
26-26: LGTM! Proper test event configuration.The sample emit is correctly configured using the imported test events.
components/printful/sources/common/base.mjs (1)
11-15:⚠️ Potential issueAdd required metadata to alert prop.
The alert prop is missing required metadata according to component guidelines.
alert: { type: "alert", + label: "Webhook Configuration Warning", + description: "Important information about webhook URL limitations", alertType: "warning", content: "**Note** that only one webhook URL can be active for a store, so all existing webhook configuration will be disabled upon creating this source.", },Likely invalid or redundant comment.
🧰 Tools
🪛 GitHub Check: Lint Code Base
[warning] 11-11:
Component prop alert must have a label. See https://pipedream.com/docs/components/guidelines/#props
[warning] 11-11:
Component prop alert must have a description. See https://pipedream.com/docs/components/guidelines/#propscomponents/printful/sources/new-order-instant/test-event.mjs (3)
152-181: Great coverage of # scenarios!The test event effectively covers various # aspects including:
- Multiple fee types
- Loss-making scenarios
- Different currency representations
31-86: Review file handling configuration.Several concerns in the file configuration:
- The file size (45MB) is quite large for a test event
- The image URLs use example.com which won't work
- The DPI setting might not represent common use cases
7-15: Verify handling of special characters in shipping_service_name.The shipping service name contains parentheses which might need proper escaping when used in different contexts (e.g., SQL queries, XML/HTML).
Resolves #15140.
Summary by CodeRabbit
New Features
Improvements
Infrastructure