Quickbooks Sync to Salesforce - #65
Open
mugesram wants to merge 10 commits into
Open
Conversation
|
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a QuickBooks→Salesforce sync integration: webhook receiver, QuickBooks HTTP client, Salesforce OAuth client, types and mapping, orchestration with conflict-resolution and parent-child handling, configuration/schema, docs, and deployment metadata. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Webhook as "Webhook Listener"
participant QB as "QuickBooks API"
participant Mapper as "Data Mapper"
participant SF as "Salesforce API"
Client->>Webhook: POST /quickbooks/webhook (QB event)
Webhook->>Webhook: verify signature & parse payload
loop per Customer entity
Webhook->>QB: fetchQuickBooksCustomerDetails(id)
QB-->>Webhook: QuickBooksCustomer
Webhook->>Mapper: mapQuickBooksCustomerToSalesforce(qbCustomer)
Mapper-->>Webhook: SalesforceAccount
Webhook->>SF: findAccountByQuickBooksId(qbId)
alt account exists
Webhook->>Webhook: shouldUpdateAccount(existing, qbCustomer)
alt update
Webhook->>SF: Update Account
else
Webhook->>SF: Create Account (maybe materialize parent)
end
else
Webhook->>SF: Create Account (maybe materialize parent)
end
end
Webhook-->>Client: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
ballerina-integrator/quickbook_sync_salesforce/quickbooks_api_simple.bal (1)
1-1: Delete the obsolete stub file.A source file whose only content says it is no longer needed just adds noise to the package. If the implementation has moved to
quickbooks_api.bal, remove this file entirely instead of keeping a tombstone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/quickbooks_api_simple.bal` at line 1, Remove the obsolete stub file quickbook_sync_salesforce/quickbooks_api_simple.bal entirely from the repository (it only contains a tombstone comment) and ensure any build or import references to quickbooks_api_simple in project files (e.g., module lists, tests, or CI configs) are deleted or updated to use quickbooks_api.bal instead so the implementation is not referenced by the removed file.ballerina-integrator/quickbook_sync_salesforce/data_mappings.bal (1)
10-76: Extract address-building logic to reduce duplication.The billing address (lines 10-39) and shipping address (lines 47-76) construction logic are nearly identical. Consider extracting a helper function to reduce duplication and improve maintainability.
♻️ Proposed helper function
// Helper to build address string from components isolated function buildAddressString(string? line1, string? line2, string? state, string? country) returns string? { string[] addressParts = []; if line1 is string { addressParts.push(line1); } if line2 is string { addressParts.push(line2); } if state is string { addressParts.push(state); } if country is string { addressParts.push(country); } return addressParts.length() > 0 ? string:'join("\n", ...addressParts) : (); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/data_mappings.bal` around lines 10 - 76, The billing and shipping address assembly logic is duplicated; extract it into a single helper (e.g., isolated function buildAddressString(line1, line2, state, country) returns string?) and replace the duplicated blocks that build addressParts and call string:'join with calls to buildAddressString for both BillAddr and ShipAddr processing; ensure you use the existing variables billingStreet, shippingStreet, billingCity, shippingPostalCode, shippingCity, and shippingPostalCode and keep the same null checks on qbCustomer?.BillAddr and qbCustomer?.ShipAddr.ballerina-integrator/quickbook_sync_salesforce/functions.bal (1)
417-424: Contact creation errors are silently ignored.If the contact creation fails (lines 418-422), the error is not logged and the overall sync still reports success. While contact creation is optional, logging failures would aid debugging.
♻️ Proposed logging for contact errors
if sfContact is SalesforceContact { salesforce:CreationResponse|error contactResult = salesforceClient->create("Contact", sfContact); if contactResult is salesforce:CreationResponse { contactId = contactResult.id; + } else { + log:printWarn(string `Failed to create contact for account ${accountId}: ${contactResult.message()}`); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines 417 - 424, The contact creation block currently ignores errors; update the logic around salesforceClient->create("Contact", sfContact) to handle the error branch: when contactResult is an error (i.e., not salesforce:CreationResponse), log the failure with context (include sfContact identity and the error) using the existing logger/processLogger so failures are visible, while keeping success behavior that sets contactId from contactResult.id; ensure this check is applied where sfContact is SalesforceContact and does not change the optional-success flow.ballerina-integrator/quickbook_sync_salesforce/types.bal (1)
105-137: Inconsistent nullability patterns between record types.
SalesforceAccountusesstring|()with explicit nil, whileSalesforceContactusesstring?for optional fields. Both are valid, but consistency would improve readability. Also, line 126 uses()|string(reversed order) unlike other fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/types.bal` around lines 105 - 137, The record types use inconsistent optional syntax: update SalesforceAccount to use the concise nullable form (string?) for all optional string fields to match SalesforceContact (e.g., change fields like Id, Site, Phone, Fax, Website, BillingStreet, BillingCity, BillingState, BillingPostalCode, BillingCountry, ShippingStreet, ShippingCity, ShippingState, ShippingPostalCode, ShippingCountry, ParentId, Description, Type, QuickbooksSync__c) and correct the reversed union on LastModifiedDate (currently ()|string) to the same nullable form (string?) so both SalesforceAccount and SalesforceContact use a consistent optional string pattern.ballerina-integrator/quickbook_sync_salesforce/connections.bal (1)
5-13: Eager vs lazy client initialization - design consideration.The Salesforce client uses eager initialization (module-level
finalwithcheck), while the QuickBooks client in quickbooks_api.bal uses lazy initialization. This is a valid design choice—eager initialization ensures fail-fast if Salesforce credentials are invalid. However, for consistency, consider documenting this intentional asymmetry or aligning both clients to use the same pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/connections.bal` around lines 5 - 13, The module currently creates an eager, module-level salesforce:Client named salesforceClient using a checked constructor (final salesforce:Client salesforceClient = check new {...}) while the QuickBooks client in quickbooks_api.bal is lazily initialized; either make the pattern consistent or document the intentional difference. To fix, choose one: (A) convert the eager initialization to a lazy factory similar to the QuickBooks pattern by moving client creation into a getSalesforceClient() (or ensureSalesforceClient()) function that constructs and returns the client (perform the check inside that function and cache the instance), referencing salesforceClient creation logic; or (B) add a clear module-level comment near the salesforce:Client salesforceClient declaration explaining the intentional fail-fast eager initialization and why it differs from the QuickBooks lazy approach so reviewers understand the design choice.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.json`:
- Around line 230-233: Two occurrences of invalid JSON Schema use an empty
string for "type" inside the anyOf branches; open the two anyOf branches where
the snippet shows { "type": "" } and replace each with { "type": "null" } so the
schema uses Draft-07 valid nullable types; update both instances in the
config-schema.json anyOf entries (the two `{ "type": "" }` objects) to `{
"type": "null" }`.
In `@ballerina-integrator/quickbook_sync_salesforce/.choreo/diagram.md`:
- Around line 19-22: The decision node O currently checks "QuickbooksSync__c
Field Exists ?" after node N already created the account, which reverses the
intended logic; change the branch condition so O reflects whether the create at
N failed specifically due to the missing QuickbooksSync__c field (e.g., "Create
failed due to missing QuickbooksSync__c?") and wire O -- Yes --> P["Fallback:
Retry Create Without Custom Field"] and O -- No --> M(["Complete"]) so only a
failed create flows to the fallback.
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal`:
- Around line 127-147: The recursive parent-account creation can overflow the
stack; modify syncCustomerToSalesforce to accept an optional depth parameter
(e.g., int currentDepth = 0) and enforce a MAX_DEPTH constant, incrementing
currentDepth on each recursive call and failing gracefully if currentDepth >=
MAX_DEPTH; update the places that call syncCustomerToSalesforce (the recursive
call in the parent-handling block and any other callers) to pass/increment this
depth, and when the limit is reached return a SyncResult failure (or error) so
fetchQuickBooksCustomerDetails + parent handling can log and stop further
recursion.
- Around line 21-25: The SOQL injection risk comes from directly interpolating
the external quickbooksId into soqlQuery inside the function
findAccountByQuickBooksId; validate quickbooksId before building the query
(e.g., ensure it matches a strict regex like only alphanumeric characters and
optional hyphens/underscores as allowed) and return an error or sanitize/reject
the value if validation fails, then only construct the soqlQuery string using
the validated value (quickbooksId) to prevent injection.
In `@ballerina-integrator/quickbook_sync_salesforce/main.bal`:
- Around line 121-144: The current use of unchecked `check` on
notification.dataChangeEvent, changeEvent.entities, and entity.name/id/operation
can abort the whole batch; replace each `check` with guarded handling: capture
results into vars (e.g., dataChangeEventJson, entitiesJson) and if they are
error (if dataChangeEventJson is error or if entitiesJson is error) log the
error and continue to next notification/changeEvent, and for extracting fields
from `entity` use type-guards (e.g., if entity.name is string then assign to
entityName else log and continue; similarly for entity.id and entity.operation)
so malformed entries are skipped but valid ones keep processing in the
eventNotifications -> dataChangeEvents -> entities loops.
In `@ballerina-integrator/quickbook_sync_salesforce/README.md`:
- Around line 269-275: Add the language tag "text" to the two untyped fenced
code blocks in README.md (the log blocks around the "QUICKBOOKS TO SALESFORCE
SYNC SERVICE STARTING / SERVICE READY - Waiting for webhooks..." block and the
"WEBHOOK RECEIVED FROM QUICKBOOKS" block) so they become ```text fenced blocks;
this fixes MD040 linting by explicitly marking those log snippets as plain text.
- Around line 76-80: Add documentation for the quickbooksTokenUrl configuration
option (which is defined in .choreo/config-schema.json) to the QuickBooks
settings list in the README and include it in the sample Config.toml;
specifically, update the section that lists
quickbooksClientId/quickbooksClientSecret/quickbooksRefreshToken/quickbooksRealmId/quickbooksBaseUrl
to also mention quickbooksTokenUrl (and mirror the same change in the later
sample block referenced around lines 107-113) so operators can discover and
override the QuickBooks token endpoint.
- Around line 19-20: The README line "Duplicate Prevention - Stores Salesforce
Account ID in QuickBooks custom field" is reversed; update the "Duplicate
Prevention" description to state that the QuickBooks customer ID is stored on
the Salesforce field QuickbooksSync__c (i.e., Salesforce stores the QuickBooks
ID on QuickbooksSync__c to prevent duplicates), replacing the current wording
that implies the Salesforce Account ID is stored in QuickBooks.
In `@ballerina-integrator/quickbook_sync_salesforce/test_webhook.bal`:
- Line 1: Restore a minimal smoke test module for the webhook path by recreating
test_webhook.bal with lightweight tests that hit the /quickbooks/webhook handler
and assert the branching behavior; include tests for the create vs update
branches (simulate webhook payloads for new vs existing records), a parent-sync
scenario (simulate a payload where parent linkage must be created/linked), and
the custom-field fallback path (simulate missing custom field and assert
fallback behavior). Locate the webhook handler and any helper functions used in
routing (e.g., the HTTP resource for "/quickbooks/webhook", the sync function(s)
that perform createOrUpdate operations, and the custom-field resolution logic)
and call them or exercise them via HTTP test clients so the test covers
verification, branching, and fallback without requiring full integration
dependencies. Ensure the file is committed under the original test module name
(test_webhook.bal) and keeps tests small and fast (smoke-level) with clear
assertions for each critical branch.
In `@ballerina-integrator/quickbook_sync_salesforce/types.bal`:
- Around line 8-27: QuickBooksWebhookEvent and DataChangeEvent type signatures
are wrong: change QuickBooksWebhookEvent.eventNotifications from string to
EventNotification[] to reflect the JSON array payload, and change
DataChangeEvent.entities from string[] to Entity[] so entities are typed as the
Entity record; update the types QuickBooksWebhookEvent, EventNotification,
DataChangeEvent (and keep Entity as-is) in types.bal to match the actual webhook
structure used in main.bal.
---
Nitpick comments:
In `@ballerina-integrator/quickbook_sync_salesforce/connections.bal`:
- Around line 5-13: The module currently creates an eager, module-level
salesforce:Client named salesforceClient using a checked constructor (final
salesforce:Client salesforceClient = check new {...}) while the QuickBooks
client in quickbooks_api.bal is lazily initialized; either make the pattern
consistent or document the intentional difference. To fix, choose one: (A)
convert the eager initialization to a lazy factory similar to the QuickBooks
pattern by moving client creation into a getSalesforceClient() (or
ensureSalesforceClient()) function that constructs and returns the client
(perform the check inside that function and cache the instance), referencing
salesforceClient creation logic; or (B) add a clear module-level comment near
the salesforce:Client salesforceClient declaration explaining the intentional
fail-fast eager initialization and why it differs from the QuickBooks lazy
approach so reviewers understand the design choice.
In `@ballerina-integrator/quickbook_sync_salesforce/data_mappings.bal`:
- Around line 10-76: The billing and shipping address assembly logic is
duplicated; extract it into a single helper (e.g., isolated function
buildAddressString(line1, line2, state, country) returns string?) and replace
the duplicated blocks that build addressParts and call string:'join with calls
to buildAddressString for both BillAddr and ShipAddr processing; ensure you use
the existing variables billingStreet, shippingStreet, billingCity,
shippingPostalCode, shippingCity, and shippingPostalCode and keep the same null
checks on qbCustomer?.BillAddr and qbCustomer?.ShipAddr.
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal`:
- Around line 417-424: The contact creation block currently ignores errors;
update the logic around salesforceClient->create("Contact", sfContact) to handle
the error branch: when contactResult is an error (i.e., not
salesforce:CreationResponse), log the failure with context (include sfContact
identity and the error) using the existing logger/processLogger so failures are
visible, while keeping success behavior that sets contactId from
contactResult.id; ensure this check is applied where sfContact is
SalesforceContact and does not change the optional-success flow.
In `@ballerina-integrator/quickbook_sync_salesforce/quickbooks_api_simple.bal`:
- Line 1: Remove the obsolete stub file
quickbook_sync_salesforce/quickbooks_api_simple.bal entirely from the repository
(it only contains a tombstone comment) and ensure any build or import references
to quickbooks_api_simple in project files (e.g., module lists, tests, or CI
configs) are deleted or updated to use quickbooks_api.bal instead so the
implementation is not referenced by the removed file.
In `@ballerina-integrator/quickbook_sync_salesforce/types.bal`:
- Around line 105-137: The record types use inconsistent optional syntax: update
SalesforceAccount to use the concise nullable form (string?) for all optional
string fields to match SalesforceContact (e.g., change fields like Id, Site,
Phone, Fax, Website, BillingStreet, BillingCity, BillingState,
BillingPostalCode, BillingCountry, ShippingStreet, ShippingCity, ShippingState,
ShippingPostalCode, ShippingCountry, ParentId, Description, Type,
QuickbooksSync__c) and correct the reversed union on LastModifiedDate (currently
()|string) to the same nullable form (string?) so both SalesforceAccount and
SalesforceContact use a consistent optional string pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c57558e-11e8-4625-b874-104f4ec8dbf7
📒 Files selected for processing (16)
ballerina-integrator/quickbook_sync_salesforce/.choreo/component.yamlballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.jsonballerina-integrator/quickbook_sync_salesforce/.choreo/diagram.mdballerina-integrator/quickbook_sync_salesforce/.choreo/instruction.mdballerina-integrator/quickbook_sync_salesforce/Ballerina.tomlballerina-integrator/quickbook_sync_salesforce/README.mdballerina-integrator/quickbook_sync_salesforce/agents.balballerina-integrator/quickbook_sync_salesforce/config.balballerina-integrator/quickbook_sync_salesforce/connections.balballerina-integrator/quickbook_sync_salesforce/data_mappings.balballerina-integrator/quickbook_sync_salesforce/functions.balballerina-integrator/quickbook_sync_salesforce/main.balballerina-integrator/quickbook_sync_salesforce/quickbooks_api.balballerina-integrator/quickbook_sync_salesforce/quickbooks_api_simple.balballerina-integrator/quickbook_sync_salesforce/test_webhook.balballerina-integrator/quickbook_sync_salesforce/types.bal
Comment on lines
+230
to
+233
| { | ||
| "type": "" | ||
| } | ||
| ] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python -m pip install --quiet jsonschema
python - <<'PY'
import json
from pathlib import Path
from jsonschema import Draft7Validator, exceptions
path = Path("ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.json")
schema = json.loads(path.read_text())
try:
Draft7Validator.check_schema(schema)
print("Draft-07 schema is valid")
except exceptions.SchemaError as err:
print("Draft-07 schema is invalid:")
print(err)
raise SystemExit(1)
PYRepository: wso2/integration-samples
Length of output: 824
🏁 Script executed:
#!/bin/bash
cd ballerina-integrator/quickbook_sync_salesforce/.choreo || exit 1
# Show lines around the first issue (lines 230-235)
sed -n '228,235p' config-schema.json | cat -n
echo "---"
# Show lines around the second issue (lines 250-255)
sed -n '248,255p' config-schema.json | cat -nRepository: wso2/integration-samples
Length of output: 643
🏁 Script executed:
#!/bin/bash
cd ballerina-integrator/quickbook_sync_salesforce/.choreo || exit 1
# Show context around first issue (lines 225-240)
echo "=== First anyOf block (lines 225-240) ==="
sed -n '225,240p' config-schema.json | cat -n
echo ""
echo "=== Second anyOf block (lines 245-260) ==="
sed -n '245,260p' config-schema.json | cat -nRepository: wso2/integration-samples
Length of output: 1314
Replace the empty-string type values with "type": "null" in both anyOf branches.
Lines 231 and 251 contain "type": "", which violates Draft-07 JSON Schema. These represent optional/nullable configuration fields (either a reference type or a value, or null). The correct Draft-07 construct is "type": "null" in place of the empty string.
Fix
{
- "type": ""
+ "type": "null"
}
]
},
"httpVersion": { {
- "type": ""
+ "type": "null"
}
]
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| "type": "" | |
| } | |
| ] | |
| { | |
| "type": "null" | |
| } | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.json`
around lines 230 - 233, Two occurrences of invalid JSON Schema use an empty
string for "type" inside the anyOf branches; open the two anyOf branches where
the snippet shows { "type": "" } and replace each with { "type": "null" } so the
schema uses Draft-07 valid nullable types; update both instances in the
config-schema.json anyOf entries (the two `{ "type": "" }` objects) to `{
"type": "null" }`.
Comment on lines
+19
to
+22
| J -- No --> N["Create Salesforce Account with QuickbooksSync__c"] | ||
| N --> O{"QuickbooksSync__c Field Exists ?"} | ||
| O -- No --> P["Fallback: Retry Create Without Custom Field"] | ||
| O -- Yes --> M(["Complete"]) |
Contributor
There was a problem hiding this comment.
Fix the create-without-parent fallback branch in the flowchart.
Line 20 checks whether QuickbooksSync__c exists after Line 19 already created the account. The behavior described elsewhere is "attempt the create with the field, then fallback only if that create fails because the field is missing," so this decision node currently reads backwards.
📝 Suggested Mermaid fix
- N["Create Salesforce Account with QuickbooksSync__c"]
- N --> O{"QuickbooksSync__c Field Exists ?"}
- O -- No --> P["Fallback: Retry Create Without Custom Field"]
- O -- Yes --> M(["Complete"])
+ N["Create Salesforce Account with QuickbooksSync__c"]
+ N --> O{"Create with QuickbooksSync__c succeeded?"}
+ O -- No, field error --> P["Fallback: Retry Create Without Custom Field"]
+ O -- Yes --> M(["Complete"])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/.choreo/diagram.md` around
lines 19 - 22, The decision node O currently checks "QuickbooksSync__c Field
Exists ?" after node N already created the account, which reverses the intended
logic; change the branch condition so O reflects whether the create at N failed
specifically due to the missing QuickbooksSync__c field (e.g., "Create failed
due to missing QuickbooksSync__c?") and wire O -- Yes --> P["Fallback: Retry
Create Without Custom Field"] and O -- No --> M(["Complete"]) so only a failed
create flows to the fallback.
Comment on lines
+21
to
+25
| public isolated function findAccountByQuickBooksId(string quickbooksId) returns string?|error { | ||
|
|
||
| string soqlQuery = string `SELECT Id FROM Account WHERE QuickbooksSync__c = '${quickbooksId}' LIMIT 1`; | ||
|
|
||
| stream<record {}, error?> resultStream = check salesforceClient->query(soqlQuery); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are other SOQL queries with string interpolation in the codebase
rg -n "string \`SELECT.*\\\$\{" --type-add 'bal:*.bal' --type balRepository: wso2/integration-samples
Length of output: 433
🏁 Script executed:
# Check the function signature and callers of findAccountByQuickBooksId
rg -B5 -A10 "public isolated function findAccountByQuickBooksId" ballerina-integrator/quickbook_sync_salesforce/functions.balRepository: wso2/integration-samples
Length of output: 643
🏁 Script executed:
# Check where existingAccountId comes from (line 249 context)
sed -n '240,260p' ballerina-integrator/quickbook_sync_salesforce/functions.balRepository: wso2/integration-samples
Length of output: 980
🏁 Script executed:
# Check how quickbooksId is passed to the function
rg -n "findAccountByQuickBooksId\(" ballerina-integrator/quickbook_sync_salesforce/functions.balRepository: wso2/integration-samples
Length of output: 465
🏁 Script executed:
# Check Ballerina Salesforce client imports and available query methods
head -20 ballerina-integrator/quickbook_sync_salesforce/functions.balRepository: wso2/integration-samples
Length of output: 596
🏁 Script executed:
# Find the syncCustomerToSalesforce function signature and its entry point
sed -n '75,90p' ballerina-integrator/quickbook_sync_salesforce/functions.balRepository: wso2/integration-samples
Length of output: 700
🏁 Script executed:
# Check the main function or entry point to see how data flows from external sources
grep -n "public function\|public isolated function" ballerina-integrator/quickbook_sync_salesforce/functions.bal | head -20Repository: wso2/integration-samples
Length of output: 513
🏁 Script executed:
# Check Salesforce client documentation in the repository or comments about parameterized queries
rg -i "parameterized|prepared|escape|sanitize" ballerina-integrator/quickbook_sync_salesforce/ -A3 -B3Repository: wso2/integration-samples
Length of output: 50
🏁 Script executed:
# Check if there are any input validation functions already in the codebase
rg -n "validate|sanitize|check.*format\|check.*pattern" ballerina-integrator/quickbook_sync_salesforce/functions.balRepository: wso2/integration-samples
Length of output: 50
🏁 Script executed:
# Check if QuickBooksCustomer type definition shows the ID field
rg -n "type QuickBooksCustomer\|record.*QuickBooksCustomer" ballerina-integrator/quickbook_sync_salesforce/ -A20Repository: wso2/integration-samples
Length of output: 50
🏁 Script executed:
# Look for test files or examples that show what QuickBooks IDs look like
find ballerina-integrator/quickbook_sync_salesforce -name "*.bal" -type f -exec grep -l "QuickBooksCustomer\|Id.*=" {} \;Repository: wso2/integration-samples
Length of output: 430
🏁 Script executed:
# Check the entire file structure to understand data sources
wc -l ballerina-integrator/quickbook_sync_salesforce/functions.balRepository: wso2/integration-samples
Length of output: 132
🏁 Script executed:
# Check types.bal to see QuickBooksCustomer definition and ID format
cat ballerina-integrator/quickbook_sync_salesforce/types.balRepository: wso2/integration-samples
Length of output: 3040
🏁 Script executed:
# Check main.bal to see how data flows from external sources
cat ballerina-integrator/quickbook_sync_salesforce/main.balRepository: wso2/integration-samples
Length of output: 7464
🏁 Script executed:
# Check quickbooks_api.bal to understand ID sources
cat ballerina-integrator/quickbook_sync_salesforce/quickbooks_api.balRepository: wso2/integration-samples
Length of output: 2033
SOQL injection risk with string interpolation at line 23.
The quickbooksId parameter is sourced from external QuickBooks API data and used directly in string interpolation within the SOQL query. While QuickBooks IDs are typically numeric, there is no validation ensuring they contain only safe characters. Consider validating that quickbooksId contains only alphanumeric characters before including it in the query.
🛡️ Proposed validation
public isolated function findAccountByQuickBooksId(string quickbooksId) returns string?|error {
+ // Validate quickbooksId contains only safe characters (alphanumeric)
+ string:RegExp safeIdPattern = re `^[a-zA-Z0-9]+$`;
+ if safeIdPattern.find(quickbooksId) is () {
+ return error(string `Invalid QuickBooks ID format: ${quickbooksId}`);
+ }
string soqlQuery = string `SELECT Id FROM Account WHERE QuickbooksSync__c = '${quickbooksId}' LIMIT 1`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public isolated function findAccountByQuickBooksId(string quickbooksId) returns string?|error { | |
| string soqlQuery = string `SELECT Id FROM Account WHERE QuickbooksSync__c = '${quickbooksId}' LIMIT 1`; | |
| stream<record {}, error?> resultStream = check salesforceClient->query(soqlQuery); | |
| public isolated function findAccountByQuickBooksId(string quickbooksId) returns string?|error { | |
| // Validate quickbooksId contains only safe characters (alphanumeric) | |
| string:RegExp safeIdPattern = re `^[a-zA-Z0-9]+$`; | |
| if safeIdPattern.find(quickbooksId) is () { | |
| return error(string `Invalid QuickBooks ID format: ${quickbooksId}`); | |
| } | |
| string soqlQuery = string `SELECT Id FROM Account WHERE QuickbooksSync__c = '${quickbooksId}' LIMIT 1`; | |
| stream<record {}, error?> resultStream = check salesforceClient->query(soqlQuery); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines 21
- 25, The SOQL injection risk comes from directly interpolating the external
quickbooksId into soqlQuery inside the function findAccountByQuickBooksId;
validate quickbooksId before building the query (e.g., ensure it matches a
strict regex like only alphanumeric characters and optional hyphens/underscores
as allowed) and return an error or sanitize/reject the value if validation
fails, then only construct the soqlQuery string using the validated value
(quickbooksId) to prevent injection.
Comment on lines
+127
to
+147
| } else { | ||
| // Parent does not exist in Salesforce - create it first (recursive) | ||
| log:printInfo(string `Parent account not found in Salesforce for QuickBooks ID ${parentCustomerId}, fetching from QuickBooks...`); | ||
| QuickBooksCustomer|error parentCustomerResult = fetchQuickBooksCustomerDetails(parentCustomerId); | ||
|
|
||
| if parentCustomerResult is error { | ||
| log:printError(string `Failed to fetch parent customer ${parentCustomerId}: ${parentCustomerResult.message()}`); | ||
| } else { | ||
| QuickBooksCustomer parentCustomer = parentCustomerResult; | ||
| SyncResult parentSyncResult = syncCustomerToSalesforce(parentCustomer, "Create"); | ||
|
|
||
| if parentSyncResult.success { | ||
| string? createdParentId = parentSyncResult?.accountId; | ||
| if createdParentId is string { | ||
| sfAccount.ParentId = createdParentId; | ||
| log:printInfo(string `Created parent account ${createdParentId} for QuickBooks parent ID ${parentCustomerId}`); | ||
| } | ||
| } else { | ||
| log:printError(string `Failed to sync parent customer ${parentCustomerId}`); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Unbounded recursion for parent hierarchy creation.
The recursive calls to syncCustomerToSalesforce for parent accounts (lines 136, 192) have no depth limit. While QuickBooks hierarchies are typically shallow, deeply nested structures could cause stack overflow. Consider adding a depth parameter to guard against this.
🛡️ Proposed depth limit
-public function syncCustomerToSalesforce(QuickBooksCustomer qbCustomer, string operation) returns SyncResult {
+public function syncCustomerToSalesforce(QuickBooksCustomer qbCustomer, string operation, int depth = 0) returns SyncResult {
+ // Guard against deeply nested hierarchies
+ if depth > 10 {
+ return {
+ success: false,
+ message: "Maximum hierarchy depth exceeded",
+ errorDetails: "Parent-child hierarchy too deep (>10 levels)"
+ };
+ }
// ... existing code ...
- SyncResult parentSyncResult = syncCustomerToSalesforce(parentCustomer, "Create");
+ SyncResult parentSyncResult = syncCustomerToSalesforce(parentCustomer, "Create", depth + 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines
127 - 147, The recursive parent-account creation can overflow the stack; modify
syncCustomerToSalesforce to accept an optional depth parameter (e.g., int
currentDepth = 0) and enforce a MAX_DEPTH constant, incrementing currentDepth on
each recursive call and failing gracefully if currentDepth >= MAX_DEPTH; update
the places that call syncCustomerToSalesforce (the recursive call in the
parent-handling block and any other callers) to pass/increment this depth, and
when the limit is reached return a SyncResult failure (or error) so
fetchQuickBooksCustomerDetails + parent handling can log and stop further
recursion.
Comment on lines
+121
to
+144
| foreach json notification in eventNotifications { | ||
| json dataChangeEventJson = check notification.dataChangeEvent; | ||
| json[] dataChangeEvents = []; | ||
|
|
||
| if dataChangeEventJson is json[] { | ||
| dataChangeEvents = dataChangeEventJson; | ||
| } else { | ||
| dataChangeEvents = [dataChangeEventJson]; | ||
| } | ||
|
|
||
| foreach json changeEvent in dataChangeEvents { | ||
| json entitiesJson = check changeEvent.entities; | ||
| json[] entities = []; | ||
|
|
||
| if entitiesJson is json[] { | ||
| entities = entitiesJson; | ||
| } else { | ||
| entities = [entitiesJson]; | ||
| } | ||
|
|
||
| foreach json entity in entities { | ||
| string entityName = check entity.name; | ||
| string entityId = check entity.id; | ||
| string operation = check entity.operation; |
Contributor
There was a problem hiding this comment.
Unguarded check expressions may cause entire webhook batch to fail.
The check expressions on lines 122, 132, and 142-144 will propagate errors and fail the entire webhook processing if any single notification/entity is malformed. Consider using is error guards to log and skip malformed entries while continuing to process valid ones.
🛡️ Proposed defensive handling
foreach json notification in eventNotifications {
- json dataChangeEventJson = check notification.dataChangeEvent;
+ json|error dataChangeEventResult = notification.dataChangeEvent;
+ if dataChangeEventResult is error {
+ log:printError(string `Skipping notification - missing dataChangeEvent: ${dataChangeEventResult.message()}`);
+ continue;
+ }
+ json dataChangeEventJson = dataChangeEventResult;
json[] dataChangeEvents = [];Apply similar pattern for changeEvent.entities and entity field extractions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| foreach json notification in eventNotifications { | |
| json dataChangeEventJson = check notification.dataChangeEvent; | |
| json[] dataChangeEvents = []; | |
| if dataChangeEventJson is json[] { | |
| dataChangeEvents = dataChangeEventJson; | |
| } else { | |
| dataChangeEvents = [dataChangeEventJson]; | |
| } | |
| foreach json changeEvent in dataChangeEvents { | |
| json entitiesJson = check changeEvent.entities; | |
| json[] entities = []; | |
| if entitiesJson is json[] { | |
| entities = entitiesJson; | |
| } else { | |
| entities = [entitiesJson]; | |
| } | |
| foreach json entity in entities { | |
| string entityName = check entity.name; | |
| string entityId = check entity.id; | |
| string operation = check entity.operation; | |
| foreach json notification in eventNotifications { | |
| json? dataChangeEventJson = notification.dataChangeEvent; | |
| if dataChangeEventJson is () { | |
| log:printError("Skipping notification - missing dataChangeEvent"); | |
| continue; | |
| } | |
| json[] dataChangeEvents = []; | |
| if dataChangeEventJson is json[] { | |
| dataChangeEvents = dataChangeEventJson; | |
| } else { | |
| dataChangeEvents = [dataChangeEventJson]; | |
| } | |
| foreach json changeEvent in dataChangeEvents { | |
| json? entitiesJson = changeEvent.entities; | |
| if entitiesJson is () { | |
| log:printError("Skipping changeEvent - missing entities"); | |
| continue; | |
| } | |
| json[] entities = []; | |
| if entitiesJson is json[] { | |
| entities = entitiesJson; | |
| } else { | |
| entities = [entitiesJson]; | |
| } | |
| foreach json entity in entities { | |
| json? entityNameValue = entity.name; | |
| json? entityIdValue = entity.id; | |
| json? operationValue = entity.operation; | |
| if entityNameValue is () || entityIdValue is () || operationValue is () { | |
| log:printError("Skipping entity - missing required fields"); | |
| continue; | |
| } | |
| string entityName = entityNameValue.toString(); | |
| string entityId = entityIdValue.toString(); | |
| string operation = operationValue.toString(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/main.bal` around lines 121 -
144, The current use of unchecked `check` on notification.dataChangeEvent,
changeEvent.entities, and entity.name/id/operation can abort the whole batch;
replace each `check` with guarded handling: capture results into vars (e.g.,
dataChangeEventJson, entitiesJson) and if they are error (if dataChangeEventJson
is error or if entitiesJson is error) log the error and continue to next
notification/changeEvent, and for extracting fields from `entity` use
type-guards (e.g., if entity.name is string then assign to entityName else log
and continue; similarly for entity.id and entity.operation) so malformed entries
are skipped but valid ones keep processing in the eventNotifications ->
dataChangeEvents -> entities loops.
Comment on lines
+76
to
+80
| - `quickbooksClientId` - Your QuickBooks OAuth2 client ID | ||
| - `quickbooksClientSecret` - Your QuickBooks OAuth2 client secret | ||
| - `quickbooksRefreshToken` - Your QuickBooks OAuth2 refresh token | ||
| - `quickbooksRealmId` - Your QuickBooks Company ID | ||
| - `quickbooksBaseUrl` - QuickBooks API base URL |
Contributor
There was a problem hiding this comment.
Document quickbooksTokenUrl alongside the other QuickBooks settings.
.choreo/config-schema.json exposes quickbooksTokenUrl, but this section and the sample Config.toml omit it. If operators need to override the token endpoint, the README currently gives them no way to discover that knob.
📝 Suggested README update
- `quickbooksClientSecret` - Your QuickBooks OAuth2 client secret
- `quickbooksRefreshToken` - Your QuickBooks OAuth2 refresh token
- `quickbooksRealmId` - Your QuickBooks Company ID
+- `quickbooksTokenUrl` - QuickBooks OAuth2 token endpoint, if you need to override the default
- `quickbooksBaseUrl` - QuickBooks API base URL
- Sandbox: `https://sandbox-quickbooks.api.intuit.com/v3/company`
- Production: `https://quickbooks.api.intuit.com/v3/company` quickbooksClientSecret = "YOUR_QUICKBOOKS_CLIENT_SECRET"
quickbooksRefreshToken = "YOUR_QUICKBOOKS_REFRESH_TOKEN"
quickbooksRealmId = "YOUR_COMPANY_ID"
+quickbooksTokenUrl = "YOUR_QUICKBOOKS_TOKEN_URL"
quickbooksBaseUrl = "https://sandbox-quickbooks.api.intuit.com/v3/company"Also applies to: 107-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/README.md` around lines 76 -
80, Add documentation for the quickbooksTokenUrl configuration option (which is
defined in .choreo/config-schema.json) to the QuickBooks settings list in the
README and include it in the sample Config.toml; specifically, update the
section that lists
quickbooksClientId/quickbooksClientSecret/quickbooksRefreshToken/quickbooksRealmId/quickbooksBaseUrl
to also mention quickbooksTokenUrl (and mirror the same change in the later
sample block referenced around lines 107-113) so operators can discover and
override the QuickBooks token endpoint.
Comment on lines
+269
to
+275
| ``` | ||
| ################################################################################################### | ||
| QUICKBOOKS TO SALESFORCE SYNC SERVICE STARTING | ||
| ################################################################################################### | ||
| SERVICE READY - Waiting for webhooks... | ||
| ################################################################################################### | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add languages to the fenced log blocks.
Lines 269 and 303 are untyped fenced blocks, which is already tripping MD040. Mark them as text so the README lints cleanly.
📝 Suggested fix
-```
+```text
###################################################################################################
QUICKBOOKS TO SALESFORCE SYNC SERVICE STARTING
###################################################################################################
SERVICE READY - Waiting for webhooks...
###################################################################################################
```diff
- ```
+ ```text
###################################################################################################
WEBHOOK RECEIVED FROM QUICKBOOKS
###################################################################################################
```
Also applies to: 303-307
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 269-269: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/README.md` around lines 269 -
275, Add the language tag "text" to the two untyped fenced code blocks in
README.md (the log blocks around the "QUICKBOOKS TO SALESFORCE SYNC SERVICE
STARTING / SERVICE READY - Waiting for webhooks..." block and the "WEBHOOK
RECEIVED FROM QUICKBOOKS" block) so they become ```text fenced blocks; this
fixes MD040 linting by explicitly marking those log snippets as plain text.
| @@ -0,0 +1 @@ | |||
| // This file has been removed - test functionality not needed | |||
Contributor
There was a problem hiding this comment.
Restore at least smoke coverage for the webhook path.
This change replaces the webhook test module with a tombstone file, which leaves verification, create/update branching, and the custom-field fallback behavior unguarded in the most critical path of this integration.
If helpful, I can draft a minimal Ballerina test matrix for /quickbooks/webhook, parent sync, and the field-missing fallback paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/test_webhook.bal` at line 1,
Restore a minimal smoke test module for the webhook path by recreating
test_webhook.bal with lightweight tests that hit the /quickbooks/webhook handler
and assert the branching behavior; include tests for the create vs update
branches (simulate webhook payloads for new vs existing records), a parent-sync
scenario (simulate a payload where parent linkage must be created/linked), and
the custom-field fallback path (simulate missing custom field and assert
fallback behavior). Locate the webhook handler and any helper functions used in
routing (e.g., the HTTP resource for "/quickbooks/webhook", the sync function(s)
that perform createOrUpdate operations, and the custom-field resolution logic)
and call them or exercise them via HTTP test clients so the test covers
verification, branching, and fallback without requiring full integration
dependencies. Ensure the file is committed under the original test module name
(test_webhook.bal) and keeps tests small and fast (smoke-level) with clear
assertions for each critical branch.
Comment on lines
+8
to
+27
| // QuickBooks Customer Webhook Event | ||
| public type QuickBooksWebhookEvent record { | ||
| string eventNotifications; | ||
| }; | ||
|
|
||
| public type EventNotification record { | ||
| string realmId; | ||
| DataChangeEvent[] dataChangeEvent; | ||
| }; | ||
|
|
||
| public type DataChangeEvent record { | ||
| string[] entities; | ||
| }; | ||
|
|
||
| public type Entity record { | ||
| string name; | ||
| string id; | ||
| string operation; | ||
| string lastUpdated; | ||
| }; |
Contributor
There was a problem hiding this comment.
Webhook event type definitions don't match actual payload structure.
The type definitions for webhook events are incorrect:
- Line 10:
eventNotificationsis typed asstringbut the actual payload contains a JSON array (as processed in main.bal lines 106-119) - Line 19:
entitiesis typed asstring[]but should beEntity[]
While main.bal works by operating on raw JSON and bypassing these types, the definitions should be corrected for consistency and potential future use with type binding.
🔧 Proposed type corrections
// QuickBooks Customer Webhook Event
public type QuickBooksWebhookEvent record {
- string eventNotifications;
+ EventNotification[] eventNotifications;
};
public type EventNotification record {
string realmId;
- DataChangeEvent[] dataChangeEvent;
+ DataChangeEvent dataChangeEvent;
};
public type DataChangeEvent record {
- string[] entities;
+ Entity[] entities;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // QuickBooks Customer Webhook Event | |
| public type QuickBooksWebhookEvent record { | |
| string eventNotifications; | |
| }; | |
| public type EventNotification record { | |
| string realmId; | |
| DataChangeEvent[] dataChangeEvent; | |
| }; | |
| public type DataChangeEvent record { | |
| string[] entities; | |
| }; | |
| public type Entity record { | |
| string name; | |
| string id; | |
| string operation; | |
| string lastUpdated; | |
| }; | |
| // QuickBooks Customer Webhook Event | |
| public type QuickBooksWebhookEvent record { | |
| EventNotification[] eventNotifications; | |
| }; | |
| public type EventNotification record { | |
| string realmId; | |
| DataChangeEvent dataChangeEvent; | |
| }; | |
| public type DataChangeEvent record { | |
| Entity[] entities; | |
| }; | |
| public type Entity record { | |
| string name; | |
| string id; | |
| string operation; | |
| string lastUpdated; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/types.bal` around lines 8 -
27, QuickBooksWebhookEvent and DataChangeEvent type signatures are wrong: change
QuickBooksWebhookEvent.eventNotifications from string to EventNotification[] to
reflect the JSON array payload, and change DataChangeEvent.entities from
string[] to Entity[] so entities are typed as the Entity record; update the
types QuickBooksWebhookEvent, EventNotification, DataChangeEvent (and keep
Entity as-is) in types.bal to match the actual webhook structure used in
main.bal.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
ballerina-integrator/quickbook_sync_salesforce/functions.bal (5)
75-75: Consider validating theoperationparameter.The function accepts any string for
operation, but only handles "Create" and "Update". Invalid values would silently fall through to the Create path (theelsebranch at line 325). Adding validation would prevent unexpected behavior from invalid operation values.♻️ Suggested validation
public function syncCustomerToSalesforce(QuickBooksCustomer qbCustomer, string operation) returns SyncResult { + // Validate operation parameter + if operation != "Create" && operation != "Update" { + return { + success: false, + message: string `Invalid operation: ${operation}. Expected 'Create' or 'Update'.` + }; + } // Check if customer should be synced🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` at line 75, The function syncCustomerToSalesforce currently accepts any string for the operation parameter and treats unknown values as "Create"; add explicit validation at the start of syncCustomerToSalesforce to allow only "Create" or "Update" (or the enum/const equivalents if defined), returning an error SyncResult (or appropriate failure response) for invalid values; reference the operation parameter in syncCustomerToSalesforce and the existing Create/Update handling branches so you prevent falling through to the else (Create) branch and surface a clear validation failure instead.
107-114: Extract duplicated error-detection logic to a helper function.The same regex-based error pattern matching for detecting missing custom field errors is repeated four times (lines 107-114, 170-176, 219-225, 332-338). Consider extracting this to a helper function to improve maintainability.
♻️ Proposed helper function
// Add this helper function isolated function isMissingCustomFieldError(string errorMessage) returns boolean { string:RegExp quickbooksSyncPattern = re `QuickbooksSync__c`; string:RegExp noColumnPattern = re `No such column`; string:RegExp badRequestPattern = re `Bad Request`; boolean hasQuickbooksSyncError = quickbooksSyncPattern.find(errorMessage) is regexp:Span; boolean hasNoColumnError = noColumnPattern.find(errorMessage) is regexp:Span; boolean hasBadRequestError = badRequestPattern.find(errorMessage) is regexp:Span; return hasQuickbooksSyncError || hasNoColumnError || hasBadRequestError; }Then replace each occurrence with:
-string:RegExp quickbooksSyncPattern = re `QuickbooksSync__c`; -string:RegExp noColumnPattern = re `No such column`; -string:RegExp badRequestPattern = re `Bad Request`; - -boolean hasQuickbooksSyncError = quickbooksSyncPattern.find(errorMessage) is regexp:Span; -boolean hasNoColumnError = noColumnPattern.find(errorMessage) is regexp:Span; -boolean hasBadRequestError = badRequestPattern.find(errorMessage) is regexp:Span; - -if hasQuickbooksSyncError || hasNoColumnError || hasBadRequestError { +if isMissingCustomFieldError(errorMessage) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines 107 - 114, The duplicated regex-based error detection (quickbooksSyncPattern, noColumnPattern, badRequestPattern and their find checks) should be extracted into a single isolated helper function named isMissingCustomFieldError(string errorMessage) that returns boolean; implement that function with the three RegExp checks and return their OR, then replace the four duplicated blocks (the occurrences that compute hasQuickbooksSyncError / hasNoColumnError / hasBadRequestError and combine them) with a call to isMissingCustomFieldError(errorMessage). Ensure the helper is used in the same scope where errorMessage is available and keep the function isolated for thread-safety.
27-28: Consider handling stream errors more explicitly.The
resultStream.next()call propagates errors viacheck, but if the stream encounters an error afternext()but beforeclose(), that error may be masked. Additionally, ifnext()fails,close()is never called, potentially leaving the stream open.♻️ Suggested improvement using do-finally pattern
public isolated function findAccountByQuickBooksId(string quickbooksId) returns string?|error { string soqlQuery = string `SELECT Id FROM Account WHERE QuickbooksSync__c = '${quickbooksId}' LIMIT 1`; stream<record {}, error?> resultStream = check salesforceClient->query(soqlQuery); - record {|record {} value;|}? result = check resultStream.next(); - check resultStream.close(); + string? accountId = (); + do { + record {|record {} value;|}? result = check resultStream.next(); + if result is record {|record {} value;|} { + record {} accountRecord = result.value; + anydata idValue = accountRecord["Id"]; + if idValue is string { + accountId = idValue; + } + } + } on fail error e { + _ = resultStream.close(); + return e; + } + check resultStream.close(); + return accountId; - - if result is record {|record {} value;|} { - record {} accountRecord = result.value; - anydata idValue = accountRecord["Id"]; - if idValue is string { - return idValue; - } - } - - return (); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines 27 - 28, Ensure the stream is always closed and surface any errors from both next() and close(): wrap the call to resultStream.next() and subsequent processing in a do ... finally (or try/finally) block so resultStream.close() runs regardless of next() failing, capture and propagate/aggregate errors from both resultStream.next() and resultStream.close() instead of letting one mask the other, and update the handling around the nullable record variable (result) so failures from either operation are logged/returned appropriately; refer to resultStream.next(), resultStream.close(), and the result variable when making these changes.
364-383: Manual field copying is fragile and may miss new fields.The retry logic manually copies each field from
sfAccounttosfAccountWithoutCustomField. IfSalesforceAccounttype is extended with new fields in the future, they won't be included in the retry attempt, potentially causing data loss.Consider a more maintainable approach using record manipulation that automatically handles all fields except
QuickbooksSync__c.♻️ Alternative approach
One option is to create a helper that clones the record and removes the field:
// Remove specific field from account record map<anydata> accountMap = sfAccount.toJson().ensureType(); _ = accountMap.remove("QuickbooksSync__c"); SalesforceAccount|error sfAccountWithoutCustomField = accountMap.cloneWithType();Note: Verify this approach works with Ballerina's type system for your specific
SalesforceAccountdefinition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines 364 - 383, The manual field-by-field copy from sfAccount into sfAccountWithoutCustomField (SalesforceAccount) is brittle; instead convert sfAccount to a mutable map/JSON, remove the "QuickbooksSync__c" key, and clone it back to the SalesforceAccount type to preserve all current and future fields (use sfAccount.toJson()/toMap(), remove("QuickbooksSync__c"), then cloneWithType() or equivalent), and update the retry logic to handle the SalesforceAccount|error result and propagate/log errors as needed.
262-262: Stream close error is silently ignored.The
closeResultfromaccountStream.close()is captured but never checked. If the stream close fails, this error would be silently swallowed, potentially masking underlying issues.♻️ Suggested fix
-error? closeResult = accountStream.close(); +error? closeResult = accountStream.close(); +if closeResult is error { + log:printWarn(string `Failed to close account stream: ${closeResult.message()}`); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` at line 262, The call to accountStream.close() stores its result in closeResult but the code never checks for an error; update the code around accountStream.close() (where closeResult is declared) to test the result (e.g., "if closeResult is error") and handle it appropriately — either log the error with a clear message using the existing logger or propagate/return the error from the enclosing function (or both) so stream close failures are not silently swallowed; ensure you reference accountStream.close() and the closeResult variable when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal`:
- Around line 178-183: The Update path currently only logs lookup failures for
missing QuickbooksSync__c but the Create path aborts on the same errors; make
the Update branch behave the same by returning/throwing the error when
hasQuickbooksSyncError || hasNoColumnError || hasBadRequestError is true. In the
block that currently only calls log:printError with parentCustomerId and
errorMessage, replace the silent-continue behavior with the same
early-return/error response used in the Create flow (mirror the return/throw
logic used where parent lookup fails earlier), ensuring the variables
hasQuickbooksSyncError, hasNoColumnError, hasBadRequestError, parentCustomerId,
and errorMessage are used consistently.
- Around line 418-422: The code currently ignores errors from
salesforceClient->create("Contact", sfContact) when contactResult is not a
salesforce:CreationResponse; add explicit error handling for contactResult (the
error branch) — e.g., detect when contactResult is an error, log the error with
context (include contactResult), and either return/propagate a failure result
(instead of allowing the function to return success: true) or set success to
false and include the error details; update the block around contactResult and
contactId so that only on a successful salesforce:CreationResponse you set
contactId, and on error you handle/log/return the error immediately.
---
Nitpick comments:
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal`:
- Line 75: The function syncCustomerToSalesforce currently accepts any string
for the operation parameter and treats unknown values as "Create"; add explicit
validation at the start of syncCustomerToSalesforce to allow only "Create" or
"Update" (or the enum/const equivalents if defined), returning an error
SyncResult (or appropriate failure response) for invalid values; reference the
operation parameter in syncCustomerToSalesforce and the existing Create/Update
handling branches so you prevent falling through to the else (Create) branch and
surface a clear validation failure instead.
- Around line 107-114: The duplicated regex-based error detection
(quickbooksSyncPattern, noColumnPattern, badRequestPattern and their find
checks) should be extracted into a single isolated helper function named
isMissingCustomFieldError(string errorMessage) that returns boolean; implement
that function with the three RegExp checks and return their OR, then replace the
four duplicated blocks (the occurrences that compute hasQuickbooksSyncError /
hasNoColumnError / hasBadRequestError and combine them) with a call to
isMissingCustomFieldError(errorMessage). Ensure the helper is used in the same
scope where errorMessage is available and keep the function isolated for
thread-safety.
- Around line 27-28: Ensure the stream is always closed and surface any errors
from both next() and close(): wrap the call to resultStream.next() and
subsequent processing in a do ... finally (or try/finally) block so
resultStream.close() runs regardless of next() failing, capture and
propagate/aggregate errors from both resultStream.next() and
resultStream.close() instead of letting one mask the other, and update the
handling around the nullable record variable (result) so failures from either
operation are logged/returned appropriately; refer to resultStream.next(),
resultStream.close(), and the result variable when making these changes.
- Around line 364-383: The manual field-by-field copy from sfAccount into
sfAccountWithoutCustomField (SalesforceAccount) is brittle; instead convert
sfAccount to a mutable map/JSON, remove the "QuickbooksSync__c" key, and clone
it back to the SalesforceAccount type to preserve all current and future fields
(use sfAccount.toJson()/toMap(), remove("QuickbooksSync__c"), then
cloneWithType() or equivalent), and update the retry logic to handle the
SalesforceAccount|error result and propagate/log errors as needed.
- Line 262: The call to accountStream.close() stores its result in closeResult
but the code never checks for an error; update the code around
accountStream.close() (where closeResult is declared) to test the result (e.g.,
"if closeResult is error") and handle it appropriately — either log the error
with a clear message using the existing logger or propagate/return the error
from the enclosing function (or both) so stream close failures are not silently
swallowed; ensure you reference accountStream.close() and the closeResult
variable when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a5fd9d8a-1ff5-42b8-bb21-22b89bdd1326
📒 Files selected for processing (1)
ballerina-integrator/quickbook_sync_salesforce/functions.bal
Comment on lines
+418
to
+422
| salesforce:CreationResponse|error contactResult = salesforceClient->create("Contact", sfContact); | ||
|
|
||
| if contactResult is salesforce:CreationResponse { | ||
| contactId = contactResult.id; | ||
| } |
Contributor
There was a problem hiding this comment.
Contact creation errors are silently ignored.
If contactResult is an error, the function continues and returns success: true without any indication that contact creation failed. This could lead to missing contacts without any visibility into the failure.
♻️ Suggested improvement
if sfContact is SalesforceContact {
salesforce:CreationResponse|error contactResult = salesforceClient->create("Contact", sfContact);
if contactResult is salesforce:CreationResponse {
contactId = contactResult.id;
+ } else {
+ log:printWarn(string `Failed to create contact for account ${accountId}: ${contactResult.message()}`);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| salesforce:CreationResponse|error contactResult = salesforceClient->create("Contact", sfContact); | |
| if contactResult is salesforce:CreationResponse { | |
| contactId = contactResult.id; | |
| } | |
| salesforce:CreationResponse|error contactResult = salesforceClient->create("Contact", sfContact); | |
| if contactResult is salesforce:CreationResponse { | |
| contactId = contactResult.id; | |
| } else { | |
| log:printWarn(string `Failed to create contact for account ${accountId}: ${contactResult.message()}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines
418 - 422, The code currently ignores errors from
salesforceClient->create("Contact", sfContact) when contactResult is not a
salesforce:CreationResponse; add explicit error handling for contactResult (the
error branch) — e.g., detect when contactResult is an error, log the error with
context (include contactResult), and either return/propagate a failure result
(instead of allowing the function to return success: true) or set success to
false and include the error details; update the block around contactResult and
contactId so that only on a successful salesforce:CreationResponse you set
contactId, and on error you handle/log/return the error immediately.
hasathcharu
reviewed
Mar 13, 2026
| @@ -0,0 +1,24 @@ | |||
| # +required The configuration file schema version | |||
Contributor
There was a problem hiding this comment.
Since salesforce uses a streaming API and not a webhook, no need to have this
Author
There was a problem hiding this comment.
component.yaml file is used for quickbooks webhook
hasathcharu
reviewed
Mar 13, 2026
Comment on lines
+1
to
+4
| # QuickBooks to Salesforce Sync Flowchart | ||
|
|
||
| ```mermaid | ||
| flowchart TB |
Contributor
There was a problem hiding this comment.
Suggested change
| # QuickBooks to Salesforce Sync Flowchart | |
| ```mermaid | |
| flowchart TB |
hasathcharu
reviewed
Mar 13, 2026
Comment on lines
+44
to
+48
| classDef startNode fill:#5E8DEB,stroke:#5E8DEB,color:#FFFFFF,stroke-width:1px | ||
| classDef endNode fill:#5E8DEB,stroke:#5E8DEB,color:#FFFFFF,stroke-width:1px | ||
| classDef processNode fill:#FFFFFF,stroke:#D9D9D9,color:#222222,stroke-width:1.5px,rx:8px,ry:8px | ||
| classDef decisionNode fill:#FFF7E6,stroke:#F5A623,color:#333333,stroke-width:2px | ||
| ``` |
Contributor
There was a problem hiding this comment.
Suggested change
| classDef startNode fill:#5E8DEB,stroke:#5E8DEB,color:#FFFFFF,stroke-width:1px | |
| classDef endNode fill:#5E8DEB,stroke:#5E8DEB,color:#FFFFFF,stroke-width:1px | |
| classDef processNode fill:#FFFFFF,stroke:#D9D9D9,color:#222222,stroke-width:1.5px,rx:8px,ry:8px | |
| classDef decisionNode fill:#FFF7E6,stroke:#F5A623,color:#333333,stroke-width:2px | |
| ``` |
hasathcharu
reviewed
Mar 13, 2026
|
|
||
|
|
||
|
|
||
| ## Features |
Contributor
There was a problem hiding this comment.
This section is not required
hasathcharu
reviewed
Mar 13, 2026
Contributor
|
Prebuilt Integration Checklist
|
hasathcharu
reviewed
Mar 13, 2026
| @@ -0,0 +1 @@ | |||
| // This file is intentionally empty and can be deleted if not needed for future use | |||
Contributor
There was a problem hiding this comment.
Suggested change
| // This file is intentionally empty and can be deleted if not needed for future use |
hasathcharu
reviewed
Mar 13, 2026
| // | ||
| // QuickBooks Base URL MUST be set in Config.toml: | ||
| // - Sandbox: https://sandbox-quickbooks.api.intuit.com/v3/company | ||
| // - Production: https://quickbooks.api.intuit.com/v3/company |
Contributor
There was a problem hiding this comment.
Suggested change
| // - Production: https://quickbooks.api.intuit.com/v3/company |
hasathcharu
reviewed
Mar 13, 2026
| @@ -0,0 +1 @@ | |||
| // This file has been merged into quickbooks_api.bal and is no longer needed | |||
Contributor
There was a problem hiding this comment.
Suggested change
| // This file has been merged into quickbooks_api.bal and is no longer needed |
Better to remove this file if not needed
hasathcharu
reviewed
Mar 13, 2026
Comment on lines
+365
to
+368
|
|
||
|
|
||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
ballerina-integrator/quickbook_sync_salesforce/.choreo/instructions.md (1)
49-61: Consider documenting sync configuration options.The Additional Configurations section lists credential and webhook settings but omits the sync behavior options
conflictResolutionandfilterActiveOnlythat are defined inconfig.baland the config schema. Operators may want to customize these settings.📝 Suggested addition
3. `webhookPort`, `webhookVerifyToken` - Service port and verification token used by the QuickBooks webhook endpoint. +4. `conflictResolution` + - Strategy for handling conflicts when updating existing Salesforce accounts: SOURCE_WINS (default), DESTINATION_WINS, or MOST_RECENT. +5. `filterActiveOnly` + - Only sync active customers (default: true).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/.choreo/instructions.md` around lines 49 - 61, The Additional Configurations section omits the sync behavior options defined in config.bal; update the docs to include `conflictResolution` and `filterActiveOnly` (as they appear in the config schema and config.bal) with a short description of allowed values and defaults: explain that `conflictResolution` controls which system wins on record conflicts (e.g., "salesforce" | "quickbooks" | "lastUpdated") and `filterActiveOnly` is a boolean to limit sync to active records only. Also mention where these options are consumed (reference config.bal and any SyncManager, syncRecords, or similar functions/classes) so operators know how to customize behavior.ballerina-integrator/quickbook_sync_salesforce/types.bal (1)
126-126: Minor: Inconsistent union type ordering.Line 126 uses
()|stringwhile all other optional fields usestring|(). Consider using consistent ordering for readability.📝 Suggested fix
- ()|string LastModifiedDate?; + string|() LastModifiedDate?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/types.bal` at line 126, The optional field LastModifiedDate is declared as ()|string which is inconsistent with other optional fields; change its type annotation to string|() so the union ordering matches the project's style (update the LastModifiedDate declaration in types.bal from ()|string to string|()).ballerina-integrator/quickbook_sync_salesforce/main.bal (1)
83-86: Consider returning 400 Bad Request for malformed payloads.When the webhook payload fails JSON parsing, returning
http:INTERNAL_SERVER_ERROR(500) suggests a server-side issue. Since the problem is with the client-provided payload,http:BAD_REQUEST(400) would be more semantically correct and helps QuickBooks retry logic distinguish between recoverable server errors and permanent client errors.📝 Suggested fix
if webhookPayload is error { log:printError(string `Failed to parse webhook payload: ${webhookPayload.message()}`); - return http:INTERNAL_SERVER_ERROR; + return http:BAD_REQUEST; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/main.bal` around lines 83 - 86, The current error branch treating a failed JSON parse (webhookPayload is error) returns http:INTERNAL_SERVER_ERROR; change this to return http:BAD_REQUEST to indicate a client-side malformed payload. Update the handler where webhookPayload is validated (the "webhookPayload is error" branch and its log:printError call) to log the parsing error as before but return http:BAD_REQUEST instead of http:INTERNAL_SERVER_ERROR so QuickBooks and callers can distinguish client errors from server failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina-integrator/quickbook_sync_salesforce/README.md`:
- Line 12: Fix the duplicated word in the README sentence that currently reads
"Stores QuickBooks customer ID in Salesforce custom field for proper linking
linking" by removing the extra "linking" so the line reads "Stores QuickBooks
customer ID in Salesforce custom field for proper linking"; update the README.md
entry containing that exact sentence to remove the duplicate word.
---
Nitpick comments:
In `@ballerina-integrator/quickbook_sync_salesforce/.choreo/instructions.md`:
- Around line 49-61: The Additional Configurations section omits the sync
behavior options defined in config.bal; update the docs to include
`conflictResolution` and `filterActiveOnly` (as they appear in the config schema
and config.bal) with a short description of allowed values and defaults: explain
that `conflictResolution` controls which system wins on record conflicts (e.g.,
"salesforce" | "quickbooks" | "lastUpdated") and `filterActiveOnly` is a boolean
to limit sync to active records only. Also mention where these options are
consumed (reference config.bal and any SyncManager, syncRecords, or similar
functions/classes) so operators know how to customize behavior.
In `@ballerina-integrator/quickbook_sync_salesforce/main.bal`:
- Around line 83-86: The current error branch treating a failed JSON parse
(webhookPayload is error) returns http:INTERNAL_SERVER_ERROR; change this to
return http:BAD_REQUEST to indicate a client-side malformed payload. Update the
handler where webhookPayload is validated (the "webhookPayload is error" branch
and its log:printError call) to log the parsing error as before but return
http:BAD_REQUEST instead of http:INTERNAL_SERVER_ERROR so QuickBooks and callers
can distinguish client errors from server failures.
In `@ballerina-integrator/quickbook_sync_salesforce/types.bal`:
- Line 126: The optional field LastModifiedDate is declared as ()|string which
is inconsistent with other optional fields; change its type annotation to
string|() so the union ordering matches the project's style (update the
LastModifiedDate declaration in types.bal from ()|string to string|()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8018503-82f7-4645-af29-529ab26c269d
📒 Files selected for processing (12)
ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.jsonballerina-integrator/quickbook_sync_salesforce/.choreo/diagram.mdballerina-integrator/quickbook_sync_salesforce/.choreo/instructions.mdballerina-integrator/quickbook_sync_salesforce/Ballerina.tomlballerina-integrator/quickbook_sync_salesforce/README.mdballerina-integrator/quickbook_sync_salesforce/agents.balballerina-integrator/quickbook_sync_salesforce/config.balballerina-integrator/quickbook_sync_salesforce/connections.balballerina-integrator/quickbook_sync_salesforce/data_mappings.balballerina-integrator/quickbook_sync_salesforce/functions.balballerina-integrator/quickbook_sync_salesforce/main.balballerina-integrator/quickbook_sync_salesforce/types.bal
🚧 Files skipped from review as they are similar to previous changes (6)
- ballerina-integrator/quickbook_sync_salesforce/connections.bal
- ballerina-integrator/quickbook_sync_salesforce/Ballerina.toml
- ballerina-integrator/quickbook_sync_salesforce/agents.bal
- ballerina-integrator/quickbook_sync_salesforce/functions.bal
- ballerina-integrator/quickbook_sync_salesforce/data_mappings.bal
- ballerina-integrator/quickbook_sync_salesforce/.choreo/diagram.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.json (1)
230-233:⚠️ Potential issue | 🟠 MajorFix invalid Draft-07 nullable branches (
"type": "").Line 231 and Line 251 use an empty string for
type, which is invalid JSON Schema Draft-07 and can break config validation. Use"type": "null"for nullable cases.Proposed fix
{ - "type": "" + "type": "null" } ] }, @@ { - "type": "" + "type": "null" } ] },#!/bin/bash set -euo pipefail python -m pip install --quiet jsonschema python - <<'PY' import json import re from pathlib import Path from jsonschema import Draft7Validator, exceptions path = Path("ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.json") text = path.read_text() matches = re.findall(r'"type"\s*:\s*""', text) print(f'Found invalid empty-string type occurrences: {len(matches)}') schema = json.loads(text) try: Draft7Validator.check_schema(schema) print("Draft-07 schema is valid") except exceptions.SchemaError as err: print("Draft-07 schema is invalid:") print(err) PYAlso applies to: 250-252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.json` around lines 230 - 233, Replace the invalid Draft-07 nullable branches that use an empty string type by locating every occurrence of the JSON property '"type": ""' in config-schema.json and changing it to '"type": "null"'; ensure you update both nullable branches currently using the empty string (there are two occurrences) and re-run a Draft7 schema validation (Draft7Validator.check_schema) to confirm the schema is valid after the change.ballerina-integrator/quickbook_sync_salesforce/types.bal (1)
8-20:⚠️ Potential issue | 🟡 MinorFix the webhook record shapes.
These public types still don't match the payload shape parsed in
main.bal:eventNotificationsis not astring, andentitiesis notstring[]. Typed binding againstQuickBooksWebhookEventwill fail immediately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/types.bal` around lines 8 - 20, The public types don't match the actual webhook payload parsed in main.bal: change QuickBooksWebhookEvent.eventNotifications from string to an array of EventNotification (EventNotification[]), ensure EventNotification.dataChangeEvent is an array of DataChangeEvent (DataChangeEvent[]), and change DataChangeEvent.entities from string[] to a collection of records (e.g., record[] or a new DataChangeEntity[] type) that matches the object shape used in main.bal; update the type names QuickBooksWebhookEvent, EventNotification, and DataChangeEvent accordingly so typed binding succeeds.ballerina-integrator/quickbook_sync_salesforce/README.md (2)
67-73:⚠️ Potential issue | 🟡 MinorDocument
quickbooksTokenUrlwith the other QuickBooks settings.The runtime exposes this setting, but the QuickBooks credential list and sample
Config.tomlstill omit it, so operators cannot discover the override from the README.📝 Suggested README update
- `quickbooksClientSecret` - Your QuickBooks OAuth2 client secret - `quickbooksRefreshToken` - Your QuickBooks OAuth2 refresh token - `quickbooksRealmId` - Your QuickBooks Company ID +- `quickbooksTokenUrl` - QuickBooks OAuth2 token endpoint, if you need to override the default - `quickbooksBaseUrl` - QuickBooks API base URL - Sandbox: `https://sandbox-quickbooks.api.intuit.com/v3/company` - Production: `https://quickbooks.api.intuit.com/v3/company`quickbooksClientSecret = "YOUR_QUICKBOOKS_CLIENT_SECRET" quickbooksRefreshToken = "YOUR_QUICKBOOKS_REFRESH_TOKEN" quickbooksRealmId = "YOUR_COMPANY_ID" +quickbooksTokenUrl = "https://oauth.platform.intuit.com/oauth2/v1/tokens/bearer" quickbooksBaseUrl = "https://sandbox-quickbooks.api.intuit.com/v3/company"Also applies to: 101-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/README.md` around lines 67 - 73, The README's QuickBooks settings list omits the quickbooksTokenUrl runtime key; add a new bullet for quickbooksTokenUrl alongside quickbooksClientId/Secret/RefreshToken/RealmId/BaseUrl in the QuickBooks settings section and update the sample Config.toml to include quickbooksTokenUrl default values; document the typical sandbox and production token endpoints (e.g., sandbox and production OAuth token URLs) and ensure the same addition is made where the README lists settings again (around lines 101-105) so operators can discover and override the token URL at runtime.
294-300:⚠️ Potential issue | 🟡 MinorAdd
textto the plain log fences.These two blocks are still untyped, so markdownlint will keep flagging MD040.
📝 Suggested fix
-``` +```text ################################################################################################### QUICKBOOKS TO SALESFORCE SYNC SERVICE STARTING ################################################################################################### SERVICE READY - Waiting for webhooks... ###################################################################################################- ``` + ```text ################################################################################################### WEBHOOK RECEIVED FROM QUICKBOOKS ################################################################################################### ```Also applies to: 328-332
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/README.md` around lines 294 - 300, The README has untyped fenced code blocks causing markdownlint MD040; update the two plain fences containing the log banners (the block starting with "QUICKBOOKS TO SALESFORCE SYNC SERVICE STARTING ... SERVICE READY - Waiting for webhooks..." and the "WEBHOOK RECEIVED FROM QUICKBOOKS" block) to use language identifier "text" by changing the opening backticks from ``` to ```text so both blocks become typed code fences.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal`:
- Around line 130-147: The code continues processing when fetching or syncing
the parent customer fails, which can create or update the child as a top-level
Account or send an empty ParentId; update the logic in the block around
fetchQuickBooksCustomerDetails(parentCustomerId) and the calls to
syncCustomerToSalesforce(parentCustomer, "Create") so that if
parentCustomerResult is error or parentSyncResult.success is false you
immediately return a failed SyncResult (propagate the error message) instead of
proceeding to set sfAccount.ParentId or continuing the create/update flow;
locate and modify the handling in the fetchQuickBooksCustomerDetails /
syncCustomerToSalesforce sequence (also apply the same guard in the analogous
section around lines 186-200) to abort and return failure when the parent is
unresolved.
- Around line 107-115: The current fallback treats any error containing "Bad
Request" (badRequestPattern / hasBadRequestError) as equivalent to missing
QuickbooksSync__c and incorrectly routes unrelated 400s into the
QuickbooksSync__c branch; change the match to be field-specific by either
parsing Salesforce's structured error payload (check error code/field name) or
tightening badRequestPattern to only match messages that reference the
QuickbooksSync__c field (e.g., require "QuickbooksSync__c" or the Salesforce
field-error code in the regex), and update the conditional that uses
hasQuickbooksSyncError || hasNoColumnError || hasBadRequestError to use the new
field-specific check instead.
- Around line 21-38: Before calling salesforceClient->create() in the Create
branch, make the flow idempotent by first checking for existing Accounts with
the same Quickbooks ID: call findAccountByQuickBooksId(quickbooksId) (or run a
SOQL query via salesforceClient->query to count matches) and if count == 1
return the existing Account Id instead of creating a new one; if count > 1
return/fail with a clear error indicating duplicate Accounts found; only call
salesforceClient->create() when no existing Account matches. Use the existing
findAccountByQuickBooksId, salesforceClient->query, and salesforceClient->create
symbols to locate and update the Create branch logic.
In `@ballerina-integrator/quickbook_sync_salesforce/main.bal`:
- Around line 74-97: Add HMAC-SHA256 signature validation at the start of the
resource function post webhook: extract the raw request body bytes (instead of
immediately parsing JSON), compute HMAC-SHA256 using your verifier token as the
key, Base64-encode the digest and compare it to the intuit-signature header; if
the header is missing or the signatures do not match, log and return an error
(do not call request.getJsonPayload() or processQuickBooksWebhook). Keep the
rest of the flow (parsing payload with request.getJsonPayload(), calling
processQuickBooksWebhook, and returning http:OK/http:INTERNAL_SERVER_ERROR)
unchanged, but only after the signature check passes.
- Around line 74-97: The webhook handler resource function post webhook
currently blocks on processing; change it to validate/authenticate the request,
persist or enqueue webhookPayload (e.g., write to a DB or message queue) and
immediately return http:OK before doing any sync work, then kick off the
long-running work asynchronously (use a detached background/task via Ballerina's
start expression or a worker pool) to call processQuickBooksWebhook (and its
internals fetchQuickBooksCustomerDetails()/syncCustomerToSalesforce()) so that
validation/enqueue happens inline but fetch/sync run out-of-band; keep logging
and error handling inside the background task and ensure the resource returns
200 within the request path.
- Around line 120-149: Before unpacking dataChangeEvent in the foreach over
eventNotifications, extract and validate the notification's intuitaccountid
against the configured quickbooksRealmId; in the loop that iterates
eventNotifications, read notification.intuitaccountid (ensuring it's a string),
and if it's absent or does not equal quickbooksRealmId, log/reject the
notification and continue to the next notification instead of processing (so you
don't apply events to the wrong tenant). Add this check near the start of the
foreach (before using dataChangeEventJson and before calling
fetchQuickBooksCustomerDetails) and use quickbooksRealmId as the canonical
configured identifier when comparing.
---
Duplicate comments:
In `@ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.json`:
- Around line 230-233: Replace the invalid Draft-07 nullable branches that use
an empty string type by locating every occurrence of the JSON property '"type":
""' in config-schema.json and changing it to '"type": "null"'; ensure you update
both nullable branches currently using the empty string (there are two
occurrences) and re-run a Draft7 schema validation
(Draft7Validator.check_schema) to confirm the schema is valid after the change.
In `@ballerina-integrator/quickbook_sync_salesforce/README.md`:
- Around line 67-73: The README's QuickBooks settings list omits the
quickbooksTokenUrl runtime key; add a new bullet for quickbooksTokenUrl
alongside quickbooksClientId/Secret/RefreshToken/RealmId/BaseUrl in the
QuickBooks settings section and update the sample Config.toml to include
quickbooksTokenUrl default values; document the typical sandbox and production
token endpoints (e.g., sandbox and production OAuth token URLs) and ensure the
same addition is made where the README lists settings again (around lines
101-105) so operators can discover and override the token URL at runtime.
- Around line 294-300: The README has untyped fenced code blocks causing
markdownlint MD040; update the two plain fences containing the log banners (the
block starting with "QUICKBOOKS TO SALESFORCE SYNC SERVICE STARTING ... SERVICE
READY - Waiting for webhooks..." and the "WEBHOOK RECEIVED FROM QUICKBOOKS"
block) to use language identifier "text" by changing the opening backticks from
``` to ```text so both blocks become typed code fences.
In `@ballerina-integrator/quickbook_sync_salesforce/types.bal`:
- Around line 8-20: The public types don't match the actual webhook payload
parsed in main.bal: change QuickBooksWebhookEvent.eventNotifications from string
to an array of EventNotification (EventNotification[]), ensure
EventNotification.dataChangeEvent is an array of DataChangeEvent
(DataChangeEvent[]), and change DataChangeEvent.entities from string[] to a
collection of records (e.g., record[] or a new DataChangeEntity[] type) that
matches the object shape used in main.bal; update the type names
QuickBooksWebhookEvent, EventNotification, and DataChangeEvent accordingly so
typed binding succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1183972-951b-4f96-9b02-b01edfdd49fd
📒 Files selected for processing (14)
ballerina-integrator/quickbook_sync_salesforce/.choreo/component.yamlballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.jsonballerina-integrator/quickbook_sync_salesforce/.choreo/diagram.mdballerina-integrator/quickbook_sync_salesforce/.choreo/instructions.mdballerina-integrator/quickbook_sync_salesforce/Ballerina.tomlballerina-integrator/quickbook_sync_salesforce/README.mdballerina-integrator/quickbook_sync_salesforce/agents.balballerina-integrator/quickbook_sync_salesforce/config.balballerina-integrator/quickbook_sync_salesforce/connections.balballerina-integrator/quickbook_sync_salesforce/data_mappings.balballerina-integrator/quickbook_sync_salesforce/functions.balballerina-integrator/quickbook_sync_salesforce/main.balballerina-integrator/quickbook_sync_salesforce/quickbooks_api.balballerina-integrator/quickbook_sync_salesforce/types.bal
🚧 Files skipped from review as they are similar to previous changes (7)
- ballerina-integrator/quickbook_sync_salesforce/.choreo/diagram.md
- ballerina-integrator/quickbook_sync_salesforce/agents.bal
- ballerina-integrator/quickbook_sync_salesforce/.choreo/component.yaml
- ballerina-integrator/quickbook_sync_salesforce/connections.bal
- ballerina-integrator/quickbook_sync_salesforce/.choreo/instructions.md
- ballerina-integrator/quickbook_sync_salesforce/config.bal
- ballerina-integrator/quickbook_sync_salesforce/Ballerina.toml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
ballerina-integrator/quickbook_sync_salesforce/README.md (2)
69-77:⚠️ Potential issue | 🟡 MinorDocument the
quickbooksTokenUrlconfiguration option.A previous review identified that
.choreo/config-schema.jsonexposesquickbooksTokenUrl, but this README omits it from both the QuickBooks credentials list and the sampleConfig.toml. Operators who need to override the QuickBooks token endpoint cannot discover this option.📝 Suggested addition
Add to the QuickBooks Credentials list:
- `quickbooksClientSecret` - Your QuickBooks OAuth2 client secret - `quickbooksRefreshToken` - Your QuickBooks OAuth2 refresh token - `quickbooksRealmId` - Your QuickBooks Company ID +- `quickbooksTokenUrl` - QuickBooks OAuth2 token endpoint (optional override) - `quickbooksBaseUrl` - QuickBooks API base URLAdd to the sample Config.toml:
quickbooksClientSecret = "YOUR_QUICKBOOKS_CLIENT_SECRET" quickbooksRefreshToken = "YOUR_QUICKBOOKS_REFRESH_TOKEN" quickbooksRealmId = "YOUR_COMPANY_ID" +quickbooksTokenUrl = "YOUR_QUICKBOOKS_TOKEN_URL" quickbooksBaseUrl = "https://sandbox-quickbooks.api.intuit.com/v3/company"Also applies to: 104-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/README.md` around lines 69 - 77, The README's QuickBooks Credentials list and sample Config.toml are missing the quickbooksTokenUrl option; update the "QuickBooks Credentials" section to document `quickbooksTokenUrl` (explain it's the OAuth2 token endpoint and provide default sandbox/production values) and add the corresponding `quickbooksTokenUrl` entry with example values to the sample Config.toml block so operators can override the QuickBooks token endpoint (this aligns README with .choreo/config-schema.json).
307-313:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks.
The log output blocks are missing language identifiers, triggering MD040 linting warnings. A previous review flagged these same blocks and suggested marking them as
text.📝 Suggested fix
-``` +```text ################################################################################################### QUICKBOOKS TO SALESFORCE SYNC SERVICE STARTING ################################################################################################### SERVICE READY - Waiting for webhooks... ###################################################################################################```diff - ``` + ```text ################################################################################################### WEBHOOK RECEIVED FROM QUICKBOOKS ################################################################################################### ```Also applies to: 341-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/README.md` around lines 307 - 313, The fenced log-output code blocks in README.md (the blocks showing "QUICKBOOKS TO SALESFORCE SYNC SERVICE STARTING" and the "WEBHOOK RECEIVED FROM QUICKBOOKS" block) are missing language identifiers and trigger MD040; update both fenced code blocks to use the text language identifier (i.e., change ``` to ```text) so the lint warning is resolved. Locate the two blocks around the shown snippets and prepend "text" after the opening backticks for each fenced block.
🧹 Nitpick comments (5)
ballerina-integrator/quickbook_sync_salesforce/README.md (1)
207-222: Consolidate duplicate custom field setup instructions.The step-by-step custom field creation instructions (lines 208-214) duplicate the detailed setup already provided in lines 127-134. Maintaining identical instructions in two locations increases the risk of documentation drift.
♻️ Suggested consolidation
Replace the duplicate 7-step instructions with a concise cross-reference:
### Custom Field Requirement -**You MUST create a custom field in Salesforce:** -1. Go to Salesforce Setup → Object Manager → Account → Fields & Relationships -2. Click "New" to create a custom field -3. Field Type: Text -4. Field Label: "Quickbooks Sync" -5. Field Name: `QuickbooksSync` (API Name will be `QuickbooksSync__c`) -6. Length: 255 -7. Save and add to page layouts as needed +**You MUST create the `QuickbooksSync__c` custom field in Salesforce.** See the [Custom Field Setup](`#custom-field-setup-required`) section above for detailed creation steps. **If the custom field doesn't exist:**This keeps the setup instructions in one authoritative location while preserving the explanation of field requirements in the Sync Behavior section.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/README.md` around lines 207 - 222, Replace the duplicated 7-step Salesforce custom field creation instructions in the "Sync Behavior" area with a one-line pointer to the authoritative setup section earlier in the README; locate the duplicate detailed list (the 7-step instructions that create the QuickbooksSync / QuickbooksSync__c Text field) and replace it with a concise cross-reference such as "See 'Custom Field Setup' above for detailed creation steps" so the detailed instructions remain only in the single canonical section and the error/behavior notes keep their explanation without repeating steps.ballerina-integrator/quickbook_sync_salesforce/functions.bal (1)
287-288: SOQL injection risk in Update path query.The
existingAccountIdis used directly in string interpolation at line 287. While this ID comes from a previous Salesforce query result (making injection less likely), applying consistent validation across all SOQL queries would be safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines 287 - 288, The SOQL string interpolation uses existingAccountId directly (queryStr and salesforceClient->query) creating an injection risk; validate or sanitize existingAccountId before building queryStr — e.g., ensure it matches Salesforce ID format (15/18 alphanumeric, no special chars) and return/log an error if it fails validation, then only interpolate the validated ID into queryStr (or use a parameterized query API if available) before calling salesforceClient->query.ballerina-integrator/quickbook_sync_salesforce/main.bal (3)
152-163: Background processing lacks error recovery or retry mechanism.The
processWebhookAsyncfunction logs errors but provides no retry mechanism or dead-letter handling for failed webhook processing. If a transient error occurs (e.g., Salesforce API timeout), the event is lost. Consider implementing a retry with backoff or persisting failed events for later reprocessing.Would you like me to suggest a simple retry mechanism with exponential backoff?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/main.bal` around lines 152 - 163, The background processor processWebhookAsync currently just logs errors from processQuickBooksWebhook and drops events; add a retry-with-exponential-backoff and dead-letter persistence: wrap the call to processQuickBooksWebhook in a loop that attempts up to maxRetries (e.g., 3-5), wait with exponential backoff (baseDelay * 2^attempt) between attempts, and treat only non-retryable errors as immediate failures; if all retries fail, persist the webhookPayload and error details to a durable store (file, DB, or dead-letter queue) and update logs via log:printError including error details and retry counts so failed events can be reprocessed later.
117-122: Non-constant-time signature comparison.The string comparison at line 118 (
intuitSignature != computedSignature) is not constant-time, which could theoretically leak timing information about the signature. Consider using a constant-time comparison function for cryptographic operations.🔐 Suggested constant-time comparison
+ // Constant-time comparison to prevent timing attacks + byte[] expectedBytes = intuitSignature.toBytes(); + byte[] actualBytes = computedSignature.toBytes(); + + if expectedBytes.length() != actualBytes.length() { + log:printError("Webhook signature validation failed: signature length mismatch"); + return http:UNAUTHORIZED; + } + + int diff = 0; + foreach int i in 0 ..< expectedBytes.length() { + diff = diff | (expectedBytes[i] ^ actualBytes[i]); + } + + if diff != 0 { - if intuitSignature != computedSignature { log:printError("Webhook signature validation failed: signatures do not match"); - log:printError(string `Expected: ${computedSignature}, Received: ${intuitSignature}`); return http:UNAUTHORIZED; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/main.bal` around lines 117 - 122, Replace the non-constant-time check using "intuitSignature != computedSignature" with a constant-time comparison: add a helper function (e.g., constantTimeEquals(expected, actual)) that compares the two strings/byte arrays in constant time (iterate bytes, accumulate differences with bitwise ops, and return boolean) and use it to compare computedSignature and intuitSignature in the signature validation branch; if constantTimeEquals(computedSignature, intuitSignature) is false, log the same errors and return http:UNAUTHORIZED.
145-145: Unused future variableasyncProcessing.The
asyncProcessingfuture is declared but never used. In Ballerina, fire-and-forget patterns should use_ =to explicitly indicate the future is intentionally discarded:♻️ Suggested fix
- future<()> asyncProcessing = start processWebhookAsync(webhookPayload); + _ = start processWebhookAsync(webhookPayload);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/quickbook_sync_salesforce/main.bal` at line 145, The local future variable asyncProcessing is created by starting processWebhookAsync(webhookPayload) but never used; change the fire-and-forget to discard the future by assigning it to the blank identifier (use the underscore as the target when starting processWebhookAsync) so the intent is explicit and the unused-variable warning is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal`:
- Around line 446-465: The created SalesforceAccount object
sfAccountWithoutCustomField omits the QuickbooksSync__c custom field, so
subsequent syncs (findAccountByQuickBooksId) can't locate the record and may
create duplicates; update the sfAccountWithoutCustomField construction to
include QuickbooksSync__c (e.g., copy from sfAccount?.QuickbooksSync__c or set
it from the QuickBooks ID you have), and add a clear warning log or return a
partial-success indicator when QuickbooksSync__c is absent so callers know the
account lacks sync tracking; refer to SalesforceAccount,
sfAccountWithoutCustomField, and findAccountByQuickBooksId when making the
change.
In `@ballerina-integrator/quickbook_sync_salesforce/main.bal`:
- Around line 119-120: Remove the sensitive logging of the computed signature:
delete or stop calling log:printError with the string interpolation that prints
computedSignature and intuitSignature, and instead log a non-sensitive message
such as "Webhook signature validation failed: signatures do not match" (using
the existing log:printError) or, if you need extra context, log only redacted or
hashed forms (e.g., last N chars or a hash) of
computedSignature/intuitSignature; update the code paths that call
log:printError with computedSignature/intuitSignature so only non-sensitive
information is emitted.
---
Duplicate comments:
In `@ballerina-integrator/quickbook_sync_salesforce/README.md`:
- Around line 69-77: The README's QuickBooks Credentials list and sample
Config.toml are missing the quickbooksTokenUrl option; update the "QuickBooks
Credentials" section to document `quickbooksTokenUrl` (explain it's the OAuth2
token endpoint and provide default sandbox/production values) and add the
corresponding `quickbooksTokenUrl` entry with example values to the sample
Config.toml block so operators can override the QuickBooks token endpoint (this
aligns README with .choreo/config-schema.json).
- Around line 307-313: The fenced log-output code blocks in README.md (the
blocks showing "QUICKBOOKS TO SALESFORCE SYNC SERVICE STARTING" and the "WEBHOOK
RECEIVED FROM QUICKBOOKS" block) are missing language identifiers and trigger
MD040; update both fenced code blocks to use the text language identifier (i.e.,
change ``` to ```text) so the lint warning is resolved. Locate the two blocks
around the shown snippets and prepend "text" after the opening backticks for
each fenced block.
---
Nitpick comments:
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal`:
- Around line 287-288: The SOQL string interpolation uses existingAccountId
directly (queryStr and salesforceClient->query) creating an injection risk;
validate or sanitize existingAccountId before building queryStr — e.g., ensure
it matches Salesforce ID format (15/18 alphanumeric, no special chars) and
return/log an error if it fails validation, then only interpolate the validated
ID into queryStr (or use a parameterized query API if available) before calling
salesforceClient->query.
In `@ballerina-integrator/quickbook_sync_salesforce/main.bal`:
- Around line 152-163: The background processor processWebhookAsync currently
just logs errors from processQuickBooksWebhook and drops events; add a
retry-with-exponential-backoff and dead-letter persistence: wrap the call to
processQuickBooksWebhook in a loop that attempts up to maxRetries (e.g., 3-5),
wait with exponential backoff (baseDelay * 2^attempt) between attempts, and
treat only non-retryable errors as immediate failures; if all retries fail,
persist the webhookPayload and error details to a durable store (file, DB, or
dead-letter queue) and update logs via log:printError including error details
and retry counts so failed events can be reprocessed later.
- Around line 117-122: Replace the non-constant-time check using
"intuitSignature != computedSignature" with a constant-time comparison: add a
helper function (e.g., constantTimeEquals(expected, actual)) that compares the
two strings/byte arrays in constant time (iterate bytes, accumulate differences
with bitwise ops, and return boolean) and use it to compare computedSignature
and intuitSignature in the signature validation branch; if
constantTimeEquals(computedSignature, intuitSignature) is false, log the same
errors and return http:UNAUTHORIZED.
- Line 145: The local future variable asyncProcessing is created by starting
processWebhookAsync(webhookPayload) but never used; change the fire-and-forget
to discard the future by assigning it to the blank identifier (use the
underscore as the target when starting processWebhookAsync) so the intent is
explicit and the unused-variable warning is removed.
In `@ballerina-integrator/quickbook_sync_salesforce/README.md`:
- Around line 207-222: Replace the duplicated 7-step Salesforce custom field
creation instructions in the "Sync Behavior" area with a one-line pointer to the
authoritative setup section earlier in the README; locate the duplicate detailed
list (the 7-step instructions that create the QuickbooksSync / QuickbooksSync__c
Text field) and replace it with a concise cross-reference such as "See 'Custom
Field Setup' above for detailed creation steps" so the detailed instructions
remain only in the single canonical section and the error/behavior notes keep
their explanation without repeating steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0b771cc-82b6-422b-bb6d-0fce3ac06fdb
📒 Files selected for processing (3)
ballerina-integrator/quickbook_sync_salesforce/README.mdballerina-integrator/quickbook_sync_salesforce/functions.balballerina-integrator/quickbook_sync_salesforce/main.bal
Comment on lines
+446
to
+465
| SalesforceAccount sfAccountWithoutCustomField = { | ||
| Name: sfAccount.Name, | ||
| Site: sfAccount?.Site, | ||
| Phone: sfAccount?.Phone, | ||
| Fax: sfAccount?.Fax, | ||
| Website: sfAccount?.Website, | ||
| BillingStreet: sfAccount?.BillingStreet, | ||
| BillingCity: sfAccount?.BillingCity, | ||
| BillingState: sfAccount?.BillingState, | ||
| BillingPostalCode: sfAccount?.BillingPostalCode, | ||
| BillingCountry: sfAccount?.BillingCountry, | ||
| ShippingStreet: sfAccount?.ShippingStreet, | ||
| ShippingCity: sfAccount?.ShippingCity, | ||
| ShippingState: sfAccount?.ShippingState, | ||
| ShippingPostalCode: sfAccount?.ShippingPostalCode, | ||
| ShippingCountry: sfAccount?.ShippingCountry, | ||
| ParentId: sfAccount?.ParentId, | ||
| Description: sfAccount?.Description, | ||
| Type: sfAccount?.Type | ||
| }; |
Contributor
There was a problem hiding this comment.
Account created without QuickbooksSync__c loses sync tracking capability.
When the custom field is missing and the customer has no parent, the retry creates an account without QuickbooksSync__c. This means subsequent updates from QuickBooks won't be able to locate this account (since findAccountByQuickBooksId depends on this field), potentially creating duplicates on future syncs. Consider logging a more prominent warning or returning a partial success indicator.
🛡️ Suggested enhancement
string createdAccountId = retryResult.id;
accountId = createdAccountId;
- log:printInfo(string `Created new account ${createdAccountId} for QuickBooks customer ${qbCustomer.Id} (without QuickbooksSync__c field)`);
+ log:printWarn(string `Created account ${createdAccountId} for QuickBooks customer ${qbCustomer.Id} WITHOUT QuickbooksSync__c field - future updates will NOT be tracked. Add the custom field to Salesforce to enable full sync.`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/functions.bal` around lines
446 - 465, The created SalesforceAccount object sfAccountWithoutCustomField
omits the QuickbooksSync__c custom field, so subsequent syncs
(findAccountByQuickBooksId) can't locate the record and may create duplicates;
update the sfAccountWithoutCustomField construction to include QuickbooksSync__c
(e.g., copy from sfAccount?.QuickbooksSync__c or set it from the QuickBooks ID
you have), and add a clear warning log or return a partial-success indicator
when QuickbooksSync__c is absent so callers know the account lacks sync
tracking; refer to SalesforceAccount, sfAccountWithoutCustomField, and
findAccountByQuickBooksId when making the change.
Comment on lines
+119
to
+120
| log:printError("Webhook signature validation failed: signatures do not match"); | ||
| log:printError(string `Expected: ${computedSignature}, Received: ${intuitSignature}`); |
Contributor
There was a problem hiding this comment.
Avoid logging computed signature in production.
Line 120 logs the expected (computed) signature when validation fails. This could inadvertently expose information useful for crafting valid signatures. Consider removing or reducing the verbosity of this log.
🛡️ Suggested fix
if intuitSignature != computedSignature {
log:printError("Webhook signature validation failed: signatures do not match");
- log:printError(string `Expected: ${computedSignature}, Received: ${intuitSignature}`);
return http:UNAUTHORIZED;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina-integrator/quickbook_sync_salesforce/main.bal` around lines 119 -
120, Remove the sensitive logging of the computed signature: delete or stop
calling log:printError with the string interpolation that prints
computedSignature and intuitSignature, and instead log a non-sensitive message
such as "Webhook signature validation failed: signatures do not match" (using
the existing log:printError) or, if you need extra context, log only redacted or
hashed forms (e.g., last N chars or a hash) of
computedSignature/intuitSignature; update the code paths that call
log:printError with computedSignature/intuitSignature so only non-sensitive
information is emitted.
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new Ballerina-based integration service that ingests QuickBooks customer webhooks and syncs them into Salesforce Accounts, with configurable conflict resolution, parent/child hierarchy handling, and explicit fallback/stop behavior when the QuickbooksSync__c custom field is missing (per issue #58).
Changes:
- Added webhook listener with HMAC validation, realm (tenant) verification, and async processing.
- Implemented QuickBooks customer fetch + Salesforce Account create/update logic with conflict resolution and parent-child sync.
- Added deployment/configuration documentation and Choreo component metadata/schema.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| ballerina-integrator/quickbook_sync_salesforce/types.bal | Defines integration types (QuickBooks/Salesforce records, enums, sync result). |
| ballerina-integrator/quickbook_sync_salesforce/main.bal | Webhook HTTP services (health, verification, POST receiver), signature validation, async dispatch. |
| ballerina-integrator/quickbook_sync_salesforce/functions.bal | Core sync logic: filtering, lookup, conflict resolution, create/update/fallback/parent handling. |
| ballerina-integrator/quickbook_sync_salesforce/quickbooks_api.bal | QuickBooks HTTP client init + customer fetch by ID. |
| ballerina-integrator/quickbook_sync_salesforce/data_mappings.bal | Maps QuickBooks Customer payload into a Salesforce Account record. |
| ballerina-integrator/quickbook_sync_salesforce/connections.bal | Initializes Salesforce connector client (OAuth2 refresh token). |
| ballerina-integrator/quickbook_sync_salesforce/config.bal | Declares configurable parameters for Salesforce, QuickBooks, webhook, and sync behavior. |
| ballerina-integrator/quickbook_sync_salesforce/Ballerina.toml | Declares package metadata and Ballerina distribution target. |
| ballerina-integrator/quickbook_sync_salesforce/README.md | End-to-end setup, configuration, behavior description, troubleshooting guidance. |
| ballerina-integrator/quickbook_sync_salesforce/agents.bal | Template/placeholder file. |
| ballerina-integrator/quickbook_sync_salesforce/.choreo/instructions.md | Choreo instructions for setup/configuration. |
| ballerina-integrator/quickbook_sync_salesforce/.choreo/diagram.md | Flow diagram documenting sync decision points and fallbacks. |
| ballerina-integrator/quickbook_sync_salesforce/.choreo/config-schema.json | Choreo configuration schema for runtime parameters. |
| ballerina-integrator/quickbook_sync_salesforce/.choreo/component.yaml | Choreo component definition exposing the service endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8
to
+20
| // QuickBooks Customer Webhook Event | ||
| public type QuickBooksWebhookEvent record { | ||
| string eventNotifications; | ||
| }; | ||
|
|
||
| public type EventNotification record { | ||
| string realmId; | ||
| DataChangeEvent[] dataChangeEvent; | ||
| }; | ||
|
|
||
| public type DataChangeEvent record { | ||
| string[] entities; | ||
| }; |
There was a problem hiding this comment.
The webhook payload record types don’t match how the webhook JSON is actually processed (and QuickBooks’ documented schema). eventNotifications is defined as string, and DataChangeEvent.entities is defined as string[], but the runtime code treats these as JSON objects/arrays of entity objects (name, id, operation, etc.). Please update these types to reflect the actual structure (e.g., eventNotifications as an array of EventNotification, and entities as an array of Entity) or remove the unused/incorrect types to avoid future cloneWithType()/deserialization bugs.
| // Compare signatures | ||
| if intuitSignature != computedSignature { | ||
| log:printError("Webhook signature validation failed: signatures do not match"); | ||
| log:printError(string `Expected: ${computedSignature}, Received: ${intuitSignature}`); |
There was a problem hiding this comment.
The header signature comparison logs both the computed and received signatures on mismatch. Even though this isn’t the secret key, logging raw signatures can aid replay/debugging by an attacker and increases sensitive log surface. Consider removing these values from logs (or logging only a short prefix) and keep the error message generic.
Suggested change
| log:printError(string `Expected: ${computedSignature}, Received: ${intuitSignature}`); |
Comment on lines
+11
to
+12
| // UPDATE: Searches by QuickBooks ID, updates if found, skips if not found | ||
| // - Parent-child relationships are maintained automatically |
There was a problem hiding this comment.
The high-level SYNC LOGIC comment says Update “skips if not found”, but the implementation later falls back to Create when no matching Salesforce account is found. Please update the comment to reflect the actual behavior to prevent operator confusion during incident/debugging.
Suggested change
| // UPDATE: Searches by QuickBooks ID, updates if found, skips if not found | |
| // - Parent-child relationships are maintained automatically | |
| // UPDATE: Searches by QuickBooks ID, updates if found; if not found, falls back to creating a new account | |
| // - Parent-child relationships are maintained automatically for created and updated accounts |
Comment on lines
+210
to
+222
| json dataChangeEventJson = check notification.dataChangeEvent; | ||
| json[] dataChangeEvents = []; | ||
|
|
||
| if dataChangeEventJson is json[] { | ||
| dataChangeEvents = dataChangeEventJson; | ||
| } else { | ||
| dataChangeEvents = [dataChangeEventJson]; | ||
| } | ||
|
|
||
| foreach json changeEvent in dataChangeEvents { | ||
| json entitiesJson = check changeEvent.entities; | ||
| json[] entities = []; | ||
|
|
There was a problem hiding this comment.
Using check while extracting dataChangeEvent / entities inside the per-notification loops will abort the entire webhook processing on a single malformed notification/event. Realm ID is handled defensively already; these fields should be handled similarly (capture the error, log, and continue) so one bad entry doesn’t drop the rest of the batch.
Comment on lines
+80
to
+81
| success: false, | ||
| message: "Customer filtered out based on sync criteria" |
There was a problem hiding this comment.
When a customer is filtered out (filterActiveOnly), the function returns success: false. The caller treats any success: false as a sync failure and logs it as an error, which will create noisy false alarms for intentionally skipped customers. Consider representing “skipped” as success: true with a message, or adding an explicit status field (e.g., SUCCESS/SKIPPED/FAILED) so logging and monitoring can distinguish expected skips from real failures.
Suggested change
| success: false, | |
| message: "Customer filtered out based on sync criteria" | |
| success: true, | |
| message: "Customer skipped: filtered out based on sync criteria" |
| } | ||
|
|
||
| // Parse Salesforce date and compare | ||
| time:Utc sfTime = check time:utcFromString(sfLastModified); |
There was a problem hiding this comment.
time:utcFromString(sfLastModified) is likely to fail for Salesforce LastModifiedDate values, which commonly come back with an offset like +0000 (no colon), not RFC3339. If this parsing fails, MOST_RECENT conflict resolution will error and block updates. Consider normalizing the Salesforce timestamp to an RFC3339-compatible form or parsing it with a format that matches Salesforce’s datetime representation before comparing.
Suggested change
| time:Utc sfTime = check time:utcFromString(sfLastModified); | |
| // Salesforce often returns offsets like "+0000"; normalize to "+00:00" for RFC3339 compatibility. | |
| string normalizedSfLastModified = sfLastModified.replace("+0000", "+00:00"); | |
| time:Utc sfTime = check time:utcFromString(normalizedSfLastModified); |
| log:printInfo("Webhook validated - processing asynchronously"); | ||
|
|
||
| // Start background processing using detached worker | ||
| future<()> asyncProcessing = start processWebhookAsync(webhookPayload); |
There was a problem hiding this comment.
The asyncProcessing future is assigned but never used. If you don’t intend to observe completion, consider not binding it (or explicitly ignoring it) to avoid unused-variable warnings and to make the intent clear.
Suggested change
| future<()> asyncProcessing = start processWebhookAsync(webhookPayload); | |
| start processWebhookAsync(webhookPayload); |
Comment on lines
+71
to
+77
| resource function get webhook(@http:Query string verifyToken) returns string|http:Unauthorized { | ||
| if verifyToken == webhookVerifyToken { | ||
| log:printInfo("Webhook verification successful"); | ||
| return "Webhook verified successfully"; | ||
| } | ||
|
|
||
| log:printError("Webhook verification failed - invalid token"); |
There was a problem hiding this comment.
The webhook verification endpoint requires callers to pass webhookVerifyToken in the query string. Since this token is also used as the HMAC secret, putting it in the URL risks leaking it via access logs, proxies, browser history, etc. Consider removing this endpoint in production, using a separate non-HMAC token for manual verification, and/or requiring the token via an Authorization header instead of a query parameter.
Suggested change
| resource function get webhook(@http:Query string verifyToken) returns string|http:Unauthorized { | |
| if verifyToken == webhookVerifyToken { | |
| log:printInfo("Webhook verification successful"); | |
| return "Webhook verified successfully"; | |
| } | |
| log:printError("Webhook verification failed - invalid token"); | |
| resource function get webhook(@http:Header string authorizationHeader) returns string|http:Unauthorized { | |
| if authorizationHeader == webhookVerifyToken { | |
| log:printInfo("Webhook verification successful"); | |
| return "Webhook verified successfully"; | |
| } | |
| log:printError("Webhook verification failed - invalid or missing token"); |
Comment on lines
+416
to
+422
| string:RegExp badRequestPattern = re `Bad Request`; | ||
|
|
||
| boolean hasQuickbooksSyncError = quickbooksSyncPattern.find(errorMessage) is regexp:Span; | ||
| boolean hasNoColumnError = noColumnPattern.find(errorMessage) is regexp:Span; | ||
| boolean hasBadRequestError = badRequestPattern.find(errorMessage) is regexp:Span; | ||
|
|
||
| if (hasQuickbooksSyncError || hasNoColumnError || hasBadRequestError) && sfAccount?.QuickbooksSync__c is string { |
There was a problem hiding this comment.
The “missing QuickbooksSync__c field” detection treats any error containing the phrase “Bad Request” as a missing-field scenario. Salesforce returns “Bad Request” for many unrelated validation failures (picklist/required fields/etc.), so this can trigger incorrect fallback/stop behavior and add noisy retries. Consider tightening the check to only match specific missing-field indicators (e.g., ‘No such column’ / the field API name / structured error codes from the connector) and avoid using the generic “Bad Request” substring.
Suggested change
| string:RegExp badRequestPattern = re `Bad Request`; | |
| boolean hasQuickbooksSyncError = quickbooksSyncPattern.find(errorMessage) is regexp:Span; | |
| boolean hasNoColumnError = noColumnPattern.find(errorMessage) is regexp:Span; | |
| boolean hasBadRequestError = badRequestPattern.find(errorMessage) is regexp:Span; | |
| if (hasQuickbooksSyncError || hasNoColumnError || hasBadRequestError) && sfAccount?.QuickbooksSync__c is string { | |
| boolean hasQuickbooksSyncError = quickbooksSyncPattern.find(errorMessage) is regexp:Span; | |
| boolean hasNoColumnError = noColumnPattern.find(errorMessage) is regexp:Span; | |
| if (hasQuickbooksSyncError || hasNoColumnError) && sfAccount?.QuickbooksSync__c is string { |
Comment on lines
+116
to
+124
| if hasQuickbooksSyncError || hasNoColumnError || hasBadRequestError { | ||
| log:printError("Field not there in Salesforce. For updating and having parent customer hierarchy, 'QuickbooksSync__c' custom field should be there in Salesforce. User have to create it in Salesforce Account object"); | ||
| log:printError(string `Error finding parent account with QuickBooks ID ${parentCustomerId}: ${errorMessage}`); | ||
| // Stop sync process - parent hierarchy requires custom field | ||
| return { | ||
| success: false, | ||
| message: "Cannot sync customer with parent - QuickbooksSync__c custom field missing", | ||
| errorDetails: "Field not there in Salesforce. For updating and having parent customer hierarchy, 'QuickbooksSync__c' custom field should be there in Salesforce. User have to create it in Salesforce Account object" | ||
| }; |
There was a problem hiding this comment.
The user-facing/log-facing message about the missing Salesforce field is long, repeated in multiple places, and contains grammatical issues (“User have to create it”). Consider centralizing it as a single constant/helper and rephrasing to a shorter, actionable message (e.g., “Missing required Salesforce Account field QuickbooksSync__c; create it to enable updates and parent hierarchy”). This reduces duplication and makes logs/SyncResult errorDetails clearer.
hasathcharu
reviewed
Mar 24, 2026
Comment on lines
+30
to
+47
| <details> | ||
|
|
||
| <summary>QuickBooks Setup Guide</summary> | ||
|
|
||
| 1. A QuickBooks Online account with API access | ||
| 2. OAuth2 credentials: | ||
| - Client ID | ||
| - Client Secret | ||
| - Refresh Token | ||
| - Realm ID (Company ID) | ||
| 3. Webhook configuration: | ||
| - Public HTTPS webhook endpoint | ||
| - Webhook verification token | ||
| - Customer entity subscription | ||
|
|
||
| This integration uses refresh token flow for auth. [Learn how to set up QuickBooks OAuth](https://developer.intuit.com/app/developer/qbo/docs/develop/authentication-and-authorization/oauth-2.0). | ||
|
|
||
| </details> |
Contributor
There was a problem hiding this comment.
Suggested change
| <details> | |
| <summary>QuickBooks Setup Guide</summary> | |
| 1. A QuickBooks Online account with API access | |
| 2. OAuth2 credentials: | |
| - Client ID | |
| - Client Secret | |
| - Refresh Token | |
| - Realm ID (Company ID) | |
| 3. Webhook configuration: | |
| - Public HTTPS webhook endpoint | |
| - Webhook verification token | |
| - Customer entity subscription | |
| This integration uses refresh token flow for auth. [Learn how to set up QuickBooks OAuth](https://developer.intuit.com/app/developer/qbo/docs/develop/authentication-and-authorization/oauth-2.0). | |
| </details> | |
| <details> | |
| <summary>QuickBooks Setup Guide</summary> | |
| 1. A QuickBooks Online account | |
| 2. OAuth2 credentials: | |
| - Client ID | |
| - Client Secret | |
| - Refresh Token | |
| - Company ID (Realm ID) | |
| 3. Scopes required: | |
| - `com.intuit.quickbooks.accounting` (Accounting) | |
| This integration uses refresh token flow for auth. [Learn how to set up QuickBooks OAuth](https://developer.intuit.com/app/developer/qbo/docs/develop/authentication-and-authorization/oauth-2.0). | |
| </details> | |
| <details> | |
| <summary>QuickBooks WebHook Setup Guide</summary> | |
| 1. A QuickBooks Online account with API access | |
| 2. An OAuth2.0 app created in the Intuit Developer portal. Refer to [Intuit Documentation](https://developer.intuit.com/app/developer/qbo/docs/develop/authentication-and-authorization/oauth-2.0) for steps. | |
| The following should be done after deploying the integration, and the endpoint URL is available. | |
| 1. Configure a WebHook in [QuickBooks](https://developer.intuit.com/app/developer/qbo/docs/develop/webhooks). | |
| This integration uses refresh token flow for auth. [Learn how to set up QuickBooks OAuth](https://developer.intuit.com/app/developer/qbo/docs/develop/authentication-and-authorization/oauth-2.0). | |
| </details> |
Contributor
Can we fix these |
Contributor
|
Use below Directory Name/Ballerina.toml title: quickbooks-customer-update-to-salesforce |
gabilang
previously approved these changes
Mar 25, 2026
Contributor
Shall we fix these |
hasathcharu
reviewed
Mar 25, 2026
hasathcharu
reviewed
Mar 25, 2026
hasathcharu
reviewed
Mar 25, 2026
hasathcharu
reviewed
Mar 25, 2026
hasathcharu
reviewed
Mar 25, 2026
hasathcharu
reviewed
Mar 25, 2026
Comment on lines
+15
to
+16
| // Note: QuickBooks HTTP Client is initialized in quickbooks_api.bal | ||
| // Both clients use OAuth 2.0 with automatic token refresh |
Contributor
There was a problem hiding this comment.
No need to have an http client. use trigger.quickbooks
hasathcharu
requested changes
Apr 2, 2026
| @@ -0,0 +1,40 @@ | |||
|
|
|||
Contributor
There was a problem hiding this comment.
Can we make this diagram simpler? around 5-6 nodes are enough. Also please follow the structure in salesforceops-to-gsheets
| // Webhook Configuration Record | ||
| public type WebhookConfig record {| | ||
| int port = 8080; | ||
| string verifyToken; |
Contributor
There was a problem hiding this comment.
Add verifyToken to the QuickBooksConfig and don't make the port a configurable. Make sure the port number is set to 9090. You can hard code the port number
Comment on lines
+14
to
+18
|
|
||
|
|
||
| // QuickBooks Base URL MUST be set in Config.toml: | ||
| // - Sandbox: https://sandbox-quickbooks.api.intuit.com/v3/company | ||
|
|
Contributor
There was a problem hiding this comment.
Suggested change
| // QuickBooks Base URL MUST be set in Config.toml: | |
| // - Sandbox: https://sandbox-quickbooks.api.intuit.com/v3/company |
| return sfAccount; | ||
| } | ||
|
|
||
|
|
Comment on lines
+59
to
+60
|
|
||
|
|
hasathcharu
reviewed
Apr 6, 2026
Comment on lines
+2
to
+11
| A(["Begin"]):::startNode | ||
| B["Receive QuickBooks Customer Webhook"]:::processNode | ||
| C{"Customer Event Type?"}:::decisionNode | ||
| D["Fetch & Create Salesforce Account"]:::processNode | ||
| E["Fetch & Update Salesforce Account"]:::processNode | ||
| F(["Complete"]):::endNode | ||
|
|
||
| A --> B --> C | ||
| C -- Create --> D --> F | ||
| C -- Update --> E --> F |
Contributor
There was a problem hiding this comment.
Suggested change
| A(["Begin"]):::startNode | |
| B["Receive QuickBooks Customer Webhook"]:::processNode | |
| C{"Customer Event Type?"}:::decisionNode | |
| D["Fetch & Create Salesforce Account"]:::processNode | |
| E["Fetch & Update Salesforce Account"]:::processNode | |
| F(["Complete"]):::endNode | |
| A --> B --> C | |
| C -- Create --> D --> F | |
| C -- Update --> E --> F | |
| A(["Begin"]):::startNode | |
| B["Receive QuickBooks Customer Webhook"]:::processNode | |
| C{"Customer Event Type?"}:::decisionNode | |
| D["Fetch & Create Salesforce Account"]:::processNode | |
| E["Fetch & Update Salesforce Account"]:::processNode | |
| F(["Complete"]):::endNode | |
| A --> B --> C | |
| C -- Create --> D --> F | |
| C -- Update --> E --> F |
| @@ -0,0 +1 @@ | |||
|
|
|||
Contributor
There was a problem hiding this comment.
Suggested change
When the file is empty, no need to have an empty line.
| @@ -0,0 +1,391 @@ | |||
| # AUTO-GENERATED FILE. DO NOT MODIFY. | |||
Contributor
There was a problem hiding this comment.
Let's not push this file.
Comment on lines
+1
to
+2
| # QuickBooks to Salesforce Sync - Instructions | ||
|
|
Contributor
There was a problem hiding this comment.
Suggested change
| # QuickBooks to Salesforce Sync - Instructions |
hasathcharu
reviewed
Apr 6, 2026
Comment on lines
55
to
60
| @@ -60,6 +58,4 @@ This integration uses refresh token flow for auth. [Learn how to set up QuickBoo | |||
| - Credentials and endpoint values required to authenticate and call QuickBooks APIs. | |||
| 3. `webhookPort`, `webhookVerifyToken` | |||
| - Service port and verification token used by the QuickBooks webhook endpoint. | |||
Contributor
There was a problem hiding this comment.
These are mandatory configurations right? we can remove these since these are already discussed in separate setup instructions.
hasathcharu
approved these changes
Apr 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR delivers the QuickBooks-to-Salesforce customer sync integration as a production-ready webhook flow to address delayed/manual account updates, duplicate record risk, and hierarchy mismatches between systems. It also addresses failure scenarios when the Salesforce custom field
QuickbooksSync__cis missing by enforcing clear stop/fallback behavior.Resolves https://github.com/wso2-enterprise/integration-engineering/issues/58
Goals
Quickbooks ID.Approach
QuickbooksSync__c; if not found, fall back to create.README.md.choreo/diagram.md.choreo/instructions.mdUser stories
QuickbooksSync__cis missing so failures are actionable.Release note
Adds a real-time QuickBooks-to-Salesforce customer sync service with webhook ingestion, parent-child hierarchy support, configurable conflict resolution, duplicate-prevention mapping via
QuickbooksSync__c, and explicit fallback/error handling for missing custom-field scenarios.Documentation
README.md.choreo/instructions.mdSummary by CodeRabbit
New Features
Documentation