feat: rename Shopify New Shipment trigger and add New Fulfillment Event trigger #14902
Conversation
…nt trigger - Renamed 'New Shipment' trigger to 'New Order Fulfilled' for better clarity - Added new 'New Fulfillment Event Created' trigger - Updated descriptions and nomenclature Fixes #14873
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
|
@GovardhaneNitin is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
WalkthroughA new module has been introduced in the Shopify developer application to handle fulfillment events, specifically for emitting events when a new fulfillment event is created. This module includes methods for retrieving event topics and generating metadata related to the events. Additionally, an existing module for order fulfillment has been updated to reflect a change in nomenclature from "New Shipment" to "New Order Fulfilled," with corresponding updates to its key, name, description, and versioning. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
components/shopify_developer_app/sources/new-fulfillment-event-created/new-fulfillment-event-created.mjs (1)
17-24: Consider adding more context to event metadataWhile the current metadata implementation is functional, consider enriching the summary with more context about the fulfillment event (e.g., status, type).
generateMeta(resource) { const ts = Date.parse(resource.created_at); return { id: resource.id, - summary: `New Fulfillment Event ${resource.id}`, + summary: `New Fulfillment Event ${resource.id} (${resource.status || 'unknown status'})`, ts, }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
components/shopify_developer_app/sources/new-fulfillment-event-created/new-fulfillment-event-created.mjs(1 hunks)components/shopify_developer_app/sources/new-shipment/new-order-fulfilled.mjs(2 hunks)
🔇 Additional comments (5)
components/shopify_developer_app/sources/new-shipment/new-order-fulfilled.mjs (3)
6-9: LGTM! Clear and accurate naming
The renaming from "New Shipment" to "New Order Fulfilled" better reflects the actual functionality of emitting events when orders are fulfilled. The key, name, and description changes are consistent and clear.
21-21: LGTM! Consistent event summary
The summary message has been updated to match the new naming convention and clearly indicates what happened to the order.
10-10: Verify version number change
The version has been reset from 0.0.4 to 0.0.1. While this aligns with the component rename, consider if existing integrations might be affected by this version downgrade.
✅ Verification successful
Version reset is appropriate for renamed component
The version reset from 0.0.4 to 0.0.1 is appropriate in this case because:
- The file references found in the codebase are for a different component (shipcloud) and test files
- This is effectively a new component with the renamed functionality, and starting at 0.0.1 follows semantic versioning principles
- Other Shopify components in the same directory follow similar versioning patterns (0.0.x)
- No existing installations or dependencies were found referencing the old component version
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any existing installations using the old version
rg -l "shopify_developer_app-new-shipment.*0\.0\.[1-4]"
Length of output: 57
Script:
#!/bin/bash
# Let's check for any references to the old component name and version pattern
rg "new-shipment.*version.*0\.0\.[1-4]"
# Also check for any deployment or configuration files that might reference versions
fd -e json -e yaml -e yml | xargs rg "new-shipment"
# Check git history for the version changes in this file
git log -p -- components/shopify_developer_app/sources/new-shipment/new-order-fulfilled.mjs
Length of output: 93
Script:
#!/bin/bash
# Let's check the old location of the file
fd "new-shipment.mjs"
# Check for any references to the component in the codebase
rg -l "new-shipment"
# Look for any version references in the component directory
rg "version.*0\.0\." components/shopify_developer_app/sources/
Length of output: 1997
components/shopify_developer_app/sources/new-fulfillment-event-created/new-fulfillment-event-created.mjs (2)
1-26: LGTM! Well-structured component
The implementation follows Pipedream's component guidelines:
- Reuses common webhook functionality
- Implements proper deduplication
- Uses correct event topic handling
- Generates appropriate metadata
14-16: Verify EVENT_TOPIC constant
Ensure that the FULFILLMENT_EVENTS_CREATE topic matches Shopify's webhook topic for fulfillment events.
✅ Verification successful
EVENT_TOPIC constant is correctly defined and used
The FULFILLMENT_EVENTS_CREATE topic is correctly defined in the constants file as "fulfillment_events/create", which matches Shopify's webhook topic for fulfillment events. The implementation in the new-fulfillment-event-created.mjs file correctly references this constant.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the constants definition
rg -A 1 "FULFILLMENT_EVENTS_CREATE.*=" components/shopify_developer_app/sources/common/
Length of output: 89
Script:
#!/bin/bash
# Let's try searching for constants file in the component directory and its subdirectories
fd constants.mjs components/shopify_developer_app -x cat {}
# Also search for any file that might define EVENT_TOPIC
rg -l "EVENT_TOPIC" components/shopify_developer_app
Length of output: 5836
Testing Summary: Testing Checklist Completed:
Additional Notes:
All tests completed successfully. Ready for final review. Tested by: @GovardhaneNitin |
|
Hi @GovardhaneNitin (cc @michelle0927), I believe there's a duplication of work here. Previously @michelle0927 has implemented the feature and already merge it: https://github.com/PipedreamHQ/pipedream/pull/14894/files @michelle0927, could you review the PR to confirm if it is still needed? |
Hi @GovardhaneNitin. I apologize, I didn't realize you were working on this. I'll close this PR since the changes have already been made. |
Fixes #14873
Changes made:
Testing:
Summary by CodeRabbit
New Features
Updates