Quickbooks Sandbox - new components #17078
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughNew action modules have been added to the QuickBooks Sandbox integration, each adapting corresponding actions from the main QuickBooks integration for sandbox use. These modules import the base action, adjust property definitions for the sandbox environment, and export customized action objects. The package version and dependency on the main QuickBooks package were also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SandboxAction
participant SandboxApp
participant CommonAction
User->>SandboxAction: Invoke action (e.g., create-estimate)
SandboxAction->>CommonAction: Use base action logic
SandboxAction->>SandboxApp: Use sandbox app instance
CommonAction-->>SandboxAction: Return result
SandboxAction-->>User: Return sandbox-adapted result
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/quickbooks_sandbox/actions/create-estimate/create-estimate.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/quickbooks_sandbox/actions/create-purchase-order/create-purchase-order.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/quickbooks_sandbox/actions/send-estimate/send-estimate.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
components/quickbooks_sandbox/actions/create-purchase-order/create-purchase-order.mjs (1)
18-21: Samequickbooks-prop overwrite risk as noted invoid-invoice.mjs
See earlier comment – apply the same ordering fix.components/quickbooks_sandbox/actions/send-estimate/send-estimate.mjs (1)
18-21: Samequickbooks-prop overwrite risk as noted invoid-invoice.mjs
See earlier comment – apply the same ordering fix.
🧹 Nitpick comments (2)
components/quickbooks_sandbox/actions/send-invoice/send-invoice.mjs (1)
11-17: Minor duplication – keep metadata single-sourcedYou extract
name,description,typeonly to re-assign them unchanged.
Since they’re already present inside...others, removing them from the destructuring would cut a few lines and the chance of drift:-const { - name, description, type, ...others -} = common; +const { ...others } = common;…and drop the explicit
name,description,typewhen exporting.
Optional, but keeps the shim ultra-thin.components/quickbooks_sandbox/actions/void-invoice/void-invoice.mjs (1)
1-22: Seven nearly-identical sandbox action wrappers – extract a helper to kill duplicationThe exact same scaffold (import app, import common, adjust props, export) is now repeated across every sandbox action module.
A tiny factory such asbuildSandboxAction(commonAction, key)insidecomponents/quickbooks_sandbox/common/utils.mjswould:
- remove boilerplate,
- guarantee consistent ordering (preventing bugs like the one above),
- centralise version & naming logic.
This keeps each action file to a two-liner and future additions virtually free.
📜 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 (8)
components/quickbooks_sandbox/actions/create-estimate/create-estimate.mjs(1 hunks)components/quickbooks_sandbox/actions/create-purchase-order/create-purchase-order.mjs(1 hunks)components/quickbooks_sandbox/actions/send-estimate/send-estimate.mjs(1 hunks)components/quickbooks_sandbox/actions/send-invoice/send-invoice.mjs(1 hunks)components/quickbooks_sandbox/actions/update-estimate/update-estimate.mjs(1 hunks)components/quickbooks_sandbox/actions/update-invoice/update-invoice.mjs(1 hunks)components/quickbooks_sandbox/actions/void-invoice/void-invoice.mjs(1 hunks)components/quickbooks_sandbox/package.json(2 hunks)
🔇 Additional comments (1)
components/quickbooks_sandbox/package.json (1)
3-3: Ensure the action-level versioning story is clearThe package version is bumped to
0.2.0, but every new action introduced in this PR keeps an internalversion: "0.0.1".
That is fine if the action version is intended to indicate per-action semver that starts from zero, but please double-check that:
- The publishing pipeline does not assume action version == package version.
- Down-stream services (catalog, docs generator, etc.) do not surface the mixed numbering in a confusing way.
If either tool expects the two to march in lock-step, align the numbers before merge.
components/quickbooks_sandbox/actions/update-estimate/update-estimate.mjs
Show resolved
Hide resolved
components/quickbooks_sandbox/actions/create-estimate/create-estimate.mjs
Show resolved
Hide resolved
michelle0927
moved this from Doing
to Ready for PR Review
in Component (Source and Action) Backlog
jcortes
moved this from Ready for PR Review
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to In QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from In QA
to Changes Required
in Component (Source and Action) Backlog
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check the test report below for more information |
michelle0927
moved this from Changes Required
to Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from Ready for QA
to Ready for Release
in Component (Source and Action) Backlog
|
Hi everyone, all test cases are passed! Ready for release! Test report |
|
/approve |
jcortes
force-pushed
the
quickbooks-sandbox-new-components
branch
from
f9772c8 to
1295b66
Compare
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
Followup PR to this issue
Summary by CodeRabbit
New Features
Chores