- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.5k
Shopify - action modifications #14894
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
 | 
| WalkthroughThe pull request introduces updates to the Shopify developer application by modifying the  Changes
 Assessment against linked issues
 Possibly related PRs
 Suggested labels
 Suggested reviewers
 
 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
 📒 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: 0
🧹 Outside diff range and nitpick comments (2)
components/shopify_developer_app/sources/new-fulfillment-event/new-fulfillment-event.mjs (1)
26-26: Add newline at end of fileAdd a newline character at the end of the file to fix the linting error.
}; +🧰 Tools
🪛 eslint
[error] 26-26: Newline required at end of file but not found.
(eol-last)
🪛 GitHub Check: Lint Code Base
[failure] 26-26:
Newline required at end of file but not foundcomponents/shopify_developer_app/sources/new-order-fulfilled/new-order-fulfilled.mjs (1)
17-23: Consider adding error handling for resource.updated_atThe
generateMetamethod assumesresource.updated_atwill always be present and valid. Consider adding error handling to ensure robustness.generateMeta(resource) { + if (!resource?.updated_at) { + throw new Error("Missing or invalid updated_at timestamp in resource"); + } const ts = Date.parse(resource.updated_at); + if (isNaN(ts)) { + throw new Error("Invalid timestamp format in resource.updated_at"); + } return { id: ts, summary: `New Fulfilled Order ${resource.id}.`, ts, }; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- components/shopify_developer_app/package.json(1 hunks)
- components/shopify_developer_app/sources/new-fulfillment-event/new-fulfillment-event.mjs(1 hunks)
- components/shopify_developer_app/sources/new-order-fulfilled/new-order-fulfilled.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/shopify_developer_app/package.json
🧰 Additional context used
🪛 eslint
components/shopify_developer_app/sources/new-fulfillment-event/new-fulfillment-event.mjs
[error] 26-26: Newline required at end of file but not found.
(eol-last)
🪛 GitHub Check: Lint Code Base
components/shopify_developer_app/sources/new-fulfillment-event/new-fulfillment-event.mjs
[failure] 26-26:
Newline required at end of file but not found
🔇 Additional comments (3)
components/shopify_developer_app/sources/new-fulfillment-event/new-fulfillment-event.mjs (2)
6-7: LGTM! Changes align with PR objectives
The renaming from "New Shipment" to "New Fulfillment Event" and the corresponding event topic change align well with the PR objectives. The changes make the functionality clearer and more accurate.
Also applies to: 15-15, 21-21
10-10: Verify version downgrade from 0.0.4 to 0.0.1
The version has been downgraded from 0.0.4 to 0.0.1. While this might be intentional due to the significant changes in functionality, version downgrades can cause issues with dependency management and should typically be avoided. Consider incrementing the version instead.
✅ Verification successful
Let me gather more specific information about the version history of this source file.
Let me try a different approach to check the component's version and related files.
Version change to 0.0.1 is appropriate for this new source
The version change from 0.0.4 to 0.0.1 is actually correct in this case. This is a completely new source component for handling fulfillment events, distinct from other components in the package. Looking at the codebase:
- The package version is 0.5.0, which is the overall version for the Shopify Developer App integration
- Individual sources and actions have their own versioning, starting at 0.0.1
- Other new sources in the codebase also start at version 0.0.1
- This follows the common practice of versioning new components from 0.0.1, regardless of other component versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check version history in package.json files
fd -e json -x grep -l "shopify_developer_app" {} \; | xargs cat
Length of output: 47591
Script:
#!/bin/bash
# Check git history for the specific file's version changes
git log -p components/shopify_developer_app/sources/new-fulfillment-event/new-fulfillment-event.mjs | grep -A 5 "^+.*version"
Length of output: 0
Script:
#!/bin/bash
# Find all files in the shopify_developer_app component directory
fd . components/shopify_developer_app -t f -x cat {} \; | grep -B 2 -A 2 "version"
Length of output: 11167
components/shopify_developer_app/sources/new-order-fulfilled/new-order-fulfilled.mjs (1)
1-26: LGTM! Implementation aligns with PR objectives
The new "New Order Fulfilled" trigger is well-implemented and follows the established patterns. The code structure, naming, and functionality align perfectly with the PR objectives.
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 #14873
Summary by CodeRabbit
New Features
@pipedream/shopify_developer_appto 0.5.0.Bug Fixes