-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Quickbooks Sandbox - Purchase created/Purchase updated #17904
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
WalkthroughTwo new source modules for "new-purchase-created" and "new-purchase-updated" events were added to the QuickBooks sandbox component, each adapting existing QuickBooks source logic for the sandbox environment. Additionally, package metadata was updated to increment the package version and update a dependency version. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SandboxSource
participant QuickBooksSandboxApp
participant CommonSourceLogic
User->>SandboxSource: Triggers new purchase event (created/updated)
SandboxSource->>QuickBooksSandboxApp: Uses sandbox app instance
SandboxSource->>CommonSourceLogic: Reuses and adapts common source logic
SandboxSource-->>User: Emits event with sandbox-specific configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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 (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
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: 2
🧹 Nitpick comments (2)
components/quickbooks_sandbox/sources/new-purchase-updated/new-purchase-updated.mjs (1)
6-10
: DRY opportunity – duplicated wrapper logic across sourcesThe destructuring +
adjustPropDefinitions
pattern is identical to thenew-purchase-created
source. Consider extracting a tiny helper (buildSandboxSource(commonSource, key)
) to avoid copy-paste and keep future maintenance to one place.components/quickbooks_sandbox/sources/new-purchase-created/new-purchase-created.mjs (1)
6-10
: Duplicate wrapper logic – extract a common factoryThis module duplicates the exact boilerplate found in
new-purchase-updated.mjs
. A small factory will make future additions trivial and reduce risk of inconsistencies (e.g. the spread-order bug above).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
components/quickbooks_sandbox/package.json
(2 hunks)components/quickbooks_sandbox/sources/new-purchase-created/new-purchase-created.mjs
(1 hunks)components/quickbooks_sandbox/sources/new-purchase-updated/new-purchase-updated.mjs
(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
📚 Learning: source names in monday.com components don't need to start with "new" if they emit events for updated...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Applied to files:
components/quickbooks_sandbox/sources/new-purchase-updated/new-purchase-updated.mjs
components/quickbooks_sandbox/sources/new-purchase-created/new-purchase-created.mjs
📚 Learning: in `components/the_magic_drip/sources/common.mjs`, when processing items in `getandprocessdata`, `sa...
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/quickbooks_sandbox/sources/new-purchase-updated/new-purchase-updated.mjs
📚 Learning: when developing pipedream components, do not add built-in node.js modules like `fs` to `package.json...
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/quickbooks_sandbox/package.json
🧬 Code Graph Analysis (2)
components/quickbooks_sandbox/sources/new-purchase-updated/new-purchase-updated.mjs (1)
components/quickbooks_sandbox/sources/new-purchase-created/new-purchase-created.mjs (2)
others
(6-8)props
(9-9)
components/quickbooks_sandbox/sources/new-purchase-created/new-purchase-created.mjs (1)
components/quickbooks_sandbox/sources/new-purchase-updated/new-purchase-updated.mjs (2)
others
(6-8)props
(9-9)
🔇 Additional comments (1)
components/quickbooks_sandbox/package.json (1)
3-3
: Compatibility scan clear – remember to smoke-test before publishing
A quick ripgrep across all.mjs
imports shows no renamed or removed exports in the jump from@pipedream/quickbooks@0.6.x
to@pipedream/quickbooks@^0.7.0
. All existingimport common from "@pipedream/quickbooks/…"
paths resolve as before.• No breaking import changes detected
• Still run the standard compatibility-check or smoke-test your actions and sources before publishing@pipedream/quickbooks_sandbox@0.3.0
components/quickbooks_sandbox/sources/new-purchase-created/new-purchase-created.mjs
Show resolved
Hide resolved
components/quickbooks_sandbox/sources/new-purchase-updated/new-purchase-updated.mjs
Show resolved
Hide resolved
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.
Hi @michelle0927 lgtm! Ready for QA!
Hi everyone, all test cases are passed! Ready for release! |
Follow-up PR to #17625 to update
quickbooks_sandbox
Summary by CodeRabbit
New Features
Chores