Add Prebuilt Integration to Export Salesforce Leads to Googlesheets - #61
Conversation
|
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 new Ballerina-based Salesforce → Google Sheets integration: project manifest and ignore rules, choreography schema/docs, configurable variables and JSON schema, OAuth client initializations, typed models and mapping, SOQL builder and time utilities, core sync automation (APPEND/FULL_REPLACE/UPSERT_BY_EMAIL, splitting, incremental sync), and state persistence. Changes
Sequence DiagramsequenceDiagram
rect rgba(200,200,255,0.5)
participant User
end
rect rgba(200,255,200,0.5)
participant Runtime as Ballerina Runtime
end
rect rgba(255,200,200,0.5)
participant SF as Salesforce API
participant GS as Google Sheets API
end
User->>Runtime: Deploy with config
Runtime->>Runtime: Load config, init clients, load lastSync
Runtime->>SF: Build and execute SOQL query
SF-->>Runtime: Return leads
Runtime->>Runtime: Map leads -> rows (fieldMapping)
Runtime->>GS: Resolve/create spreadsheet & sheet(s)
Runtime-->>Runtime: Select sync mode (APPEND / FULL_REPLACE / UPSERT_BY_EMAIL)
alt APPEND
Runtime->>GS: Append rows
else FULL_REPLACE
Runtime->>GS: Clear and write rows
else UPSERT_BY_EMAIL
Runtime->>GS: Read existing rows
GS-->>Runtime: Existing rows
Runtime->>Runtime: Match by email and merge
Runtime->>GS: Write updates
end
Runtime->>GS: Apply formatting (if enabled)
Runtime->>Runtime: Save lastSync timestamp (if enabled)
Runtime-->>User: Sync complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/config-schema.json (1)
66-68: Encode the documented config constraints in the schema.
timeframe,syncMode, andlastSyncTimestampare documented as a finite option set / ISO-8601 value, but the schema currently accepts any string. Catching those mistakes here will prevent avoidable runtime misconfigurations.🧩 Proposed schema tightening
"timeframe": { "type": "string", + "enum": [ + "ALL", + "YESTERDAY", + "LAST_WEEK", + "LAST_MONTH", + "LAST_YEAR" + ], "description": "" }, ... "lastSyncTimestamp": { "type": "string", + "pattern": "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}Z$", "description": "" }, "syncMode": { "type": "string", + "enum": [ + "APPEND", + "FULL_REPLACE", + "UPSERT_BY_EMAIL" + ], "description": "" },Also applies to: 78-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/config-schema.json` around lines 66 - 68, The JSON schema currently allows any string for the config fields; tighten it by encoding the documented constraints: add an "enum" listing allowed values for "timeframe" (the finite set documented) and for "syncMode" (the documented options), and constrain "lastSyncTimestamp" to an ISO‑8601 datetime using "format": "date-time" or a suitable "pattern" regex; update the corresponding other occurrences (the properties around the 78-84 block) to match the same enums/formats so the schema rejects invalid values at validation time.ballerina-integrator/salesforce-leads-to-googlesheets/types.bal (1)
34-40:SyncModeenum is defined but unused;syncModeconfig is a plain string.The enum provides compile-time safety but
config.baldeclaressyncModeasconfigurable string. Consider either using the enum type or removing the unused definition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/types.bal` around lines 34 - 40, The SyncMode enum is unused while syncMode is declared as a configurable string; either switch the configurable to the enum or remove the enum. To fix, update the configurable declaration (symbol: syncMode) to use type SyncMode with a sensible default (e.g., SyncMode.APPEND) so compile-time safety is enforced, and update any places reading/parsing the config to expect a SyncMode value (symbols: SyncMode, syncMode); alternatively if you prefer string config semantics, delete the unused SyncMode enum declaration to avoid dead code.ballerina-integrator/salesforce-leads-to-googlesheets/automation.bal (2)
38-57:syncModeis a free-form string; theSyncModeenum is unused.A
SyncModeenum is defined in types.bal butsyncModeconfig is declared asstringin config.bal. This means invalid values like"REPLACE"won't be caught until runtime at line 56. Consider validating against the enum values or using the enum type directly.♻️ Proposed validation approach
string effectiveSyncMode = syncMode.trim() == "" ? "APPEND" : syncMode; + +// Validate syncMode early +if effectiveSyncMode != "APPEND" && effectiveSyncMode != "FULL_REPLACE" && effectiveSyncMode != "UPSERT_BY_EMAIL" { + return error(string `Invalid syncMode: ${effectiveSyncMode}. Must be "APPEND", "FULL_REPLACE", or "UPSERT_BY_EMAIL"`); +} boolean isNewSpreadsheet = trimmedSpreadsheetId == "";This moves the validation earlier (before line 46) so invalid modes fail fast before any work is done.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/automation.bal` around lines 38 - 57, The code accepts syncMode as a plain string (symbol: syncMode) so invalid values surface late; update handling to use the SyncMode enum (symbol: SyncMode) or validate the string against that enum early: convert/parse syncMode into a SyncMode value (or return an error) before computing effectiveSyncMode and before any branch (symbols: effectiveSyncMode, isNewSpreadsheet) so invalid modes fail fast; ensure the default behavior still maps empty/whitespace to SyncMode.APPEND and adjust later branches to compare against the enum values (APPEND, FULL_REPLACE, UPSERT_BY_EMAIL) instead of raw strings.
158-159: Type narrowing:existingValuestype doesn't includeboolean|floatbutSheetRowdoes.
existingValuesis typed as(int|string|decimal)[][]from the Sheets API, butSheetRowis(int|string|decimal|boolean|float)[]. When comparing/updating rows, this type mismatch could cause issues. The conversion at lines 180-183 doesn't fully reconcile the types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/automation.bal` around lines 158 - 159, The variable existingValues (from existingRange.values) is declared as (int|string|decimal)[][] but your SheetRow type allows (int|string|decimal|boolean|float)[], causing a type mismatch during row comparisons/updates; fix it by either widening the existingValues declaration to (int|string|decimal|boolean|float)[][] or (preferably) immediately map existingRange.values into a normalized SheetRow[][] (create a small converter function that converts float->decimal where needed and preserves boolean) and use that normalized array for downstream logic in the functions that read existingRange, existingValues, and any comparison/update code that references SheetRow.ballerina-integrator/salesforce-leads-to-googlesheets/config.bal (1)
49-56: Configuration values lack runtime validation.
timeframe,syncMode, andtimezoneaccept any string value. Invalid values will only fail at runtime. Consider documenting valid values in comments or adding validation in the consuming code.The README documents valid values, but inline comments would help developers configuring the integration:
📝 Suggested inline documentation
-configurable string soqlFilter = ""; -configurable string timeframe = "ALL"; +configurable string soqlFilter = ""; // Custom SOQL WHERE clause, e.g., "Rating = 'Hot'" +configurable string timeframe = "ALL"; // Options: ALL, YESTERDAY, LAST_WEEK, LAST_MONTH, LAST_YEAR configurable boolean includeConverted = false; configurable boolean enableIncrementalSync = false; configurable string lastSyncTimestamp = ""; -configurable string syncMode = "APPEND"; +configurable string syncMode = "APPEND"; // Options: APPEND, FULL_REPLACE, UPSERT_BY_EMAIL configurable boolean enableAutoFormat = true; -configurable string splitBy = ""; +configurable string splitBy = ""; // Field name to split sheets by, e.g., "LeadSource", "Status"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/config.bal` around lines 49 - 56, Add runtime validation and inline documentation for the configurable variables so that invalid values are caught early: validate the configurable strings timeframe and syncMode (and timezone if present elsewhere) against the allowed sets documented in the README (e.g., timeframe ∈ {ALL, LAST_24_HOURS, LAST_7_DAYS, ...}; syncMode ∈ {APPEND, MERGE, REPLACE, ...}), and either reject invalid values with a clear error or coerce to a safe default at startup (use the variables timeframe, syncMode, lastSyncTimestamp, enableIncrementalSync to locate the consuming initialization code), and add brief inline comments next to each configurable declaration listing the permitted values and behavior.ballerina-integrator/salesforce-leads-to-googlesheets/functions.bal (1)
3-7: Minor:currentCivil.secondisdecimaland may include fractional seconds.
time:Civil.secondis of typedecimaland callingtoString()on it could produce values like"30.5"instead of"30". For strict ISO 8601 compliance, truncate to integer.♻️ Proposed fix
public function getCurrentTimestamp() returns string|error { time:Utc currentUtc = time:utcNow(); time:Civil currentCivil = time:utcToCivil(currentUtc); - return string `${currentCivil.year}-${currentCivil.month.toString().padZero(2)}-${currentCivil.day.toString().padZero(2)}T${currentCivil.hour.toString().padZero(2)}:${currentCivil.minute.toString().padZero(2)}:${currentCivil.second.toString().padZero(2)}Z`; + int secondsInt = <int>currentCivil.second; + return string `${currentCivil.year}-${currentCivil.month.toString().padZero(2)}-${currentCivil.day.toString().padZero(2)}T${currentCivil.hour.toString().padZero(2)}:${currentCivil.minute.toString().padZero(2)}:${secondsInt.toString().padZero(2)}Z`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/functions.bal` around lines 3 - 7, getCurrentTimestamp currently calls currentCivil.second.toString() but time:Civil.second is decimal and may include fractional seconds; truncate the fractional part of currentCivil.second (e.g., cast or floor to an int) before formatting so seconds are an integer (use the symbol currentCivil.second and the function getCurrentTimestamp to locate the code) and format that integer with padZero(2) to produce strict ISO 8601 seconds.ballerina-integrator/salesforce-leads-to-googlesheets/README.md (1)
119-131: Duplicate "Default Field Mapping" section and redundant content.Lines 85-92 already document the default field mapping. Lines 128-131 repeat this information, triggering a duplicate heading warning (MD024). The second occurrence adds no new value.
🧹 Proposed fix: Remove duplicate section
- **`splitBy`** (string, default: `""`) Split leads into multiple sheets by field value Examples: `"LeadSource"`, `"Status"`, `"Industry"` Leave empty to disable -**Default Field Mapping:** -``` -["Id", "FirstName", "LastName", "Email", "Phone", "Company", "Title", "Status", "LeadSource", "Industry", "Rating", "CreatedDate", "LastModifiedDate"] -``` - **SOQL Filtering:**🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/README.md` around lines 119 - 131, Remove the duplicated "Default Field Mapping" block that appears under the "Advanced Features" section; locate the repeated code block that lists the default fields (the same array starting with "Id", "FirstName", ...) and delete it so the README contains only the original mapping defined earlier, then ensure the "SOQL Filtering:" heading directly follows the "Advanced Features" content with correct markdown spacing; no other content changes are needed.
🤖 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/salesforce-leads-to-googlesheets/.choreo/instructions.md`:
- Around line 161-170: Update the docs to match the actual config keys and
defaults used in config.bal: state that spreadsheetId defaults to an empty
string (""), not `()`, and that the controlling option is `splitBy` (not
`splitByField`); also clarify that `tabName` defaults to "Leads" and is only
used when `splitBy` is set to "NONE". Reference the exact symbols `config.bal`,
`spreadsheetId`, `splitBy`, and `tabName` so the text aligns with the
implementation and remove any mention of `splitByField` or `()` as a default.
- Around line 122-124: The duplicate heading "## Step 4: Obtain Refresh Token"
causes ambiguous anchors and an MD024 warning; rename this heading to a unique
title (e.g., include context or a suffix like "for Google OAuth Playground" or
"— Refresh Token (Sheets)") so it no longer duplicates the earlier "## Step 4"
header; update the corresponding section title text where found in the file to
the new unique heading so anchors are distinct (look for the exact heading
string "## Step 4: Obtain Refresh Token" to locate and replace).
In `@ballerina-integrator/salesforce-leads-to-googlesheets/automation.bal`:
- Around line 62-65: The code logs the new incremental sync timestamp (via
getCurrentTimestamp) but never persists it to lastSyncTimestamp, so subsequent
runs re-fetch the same leads; after the if enableIncrementalSync block (or
replacing the log at line using getCurrentTimestamp), call a persistence routine
to save the timestamp (e.g., implement and invoke saveLastSyncTimestamp(string)
or updateConfigLastSyncTimestamp(string)) that writes the new value to the same
config/state store used to read lastSyncTimestamp on startup, and update the log
message to indicate the timestamp was persisted (or log a warning if persistence
fails) so users don’t need to manually edit lastSyncTimestamp.
- Around line 246-264: The isSheetEmpty function is hiding all getRange failures
by returning true on any error; update isSheetEmpty (and its call sites
expecting boolean|error) to propagate non-empty errors instead of masking them:
when sheetsClient->getRange returns an error, return that error (or rethrow)
rather than true, and only treat explicit, validated conditions (e.g., a
documented "notFound" or specific API response) as "empty"; keep the existing
checks on sheets:Range range and range.values but remove the unconditional
return true in the error branch so callers receive the error for handling.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/data_mappings.bal`:
- Around line 30-32: The loop over fieldMapping silently substitutes missing
keys with "" which hides misspellings or unsupported fields; before iterating,
validate every entry in fieldMapping against the known leadMap keys and fail
fast (throw an error or return a non-success result) for any unknown fieldName
so the caller knows the mapping is invalid, or alternatively extend the mapper
to support arbitrary lead fields by resolving them dynamically; update the logic
around the foreach over fieldMapping, the leadMap lookup, and the row.push usage
to enforce this validation and surface a clear error when fieldName is not
present.
- Around line 20-26: The mapping currently substitutes missing Salesforce values
with real business values (false/0/0.0); update the mapper so that when
lead?.IsConverted, lead?.NumberOfEmployees, or lead?.AnnualRevenue are
null/undefined you emit a blank cell instead of a falsy numeric/boolean value —
i.e., return an empty string (or null if your sheet writer treats null as blank)
for the "IsConverted", "NumberOfEmployees", and "AnnualRevenue" keys in the
mapping block so missing data remains blank in the exported sheet.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/functions.bal`:
- Around line 62-95: buildSoqlQuery currently concatenates user-configurable
values (fieldMapping, soqlFilter, lastSyncTimestamp) into the SOQL string
causing injection risk; fix by validating each field in fieldMapping against a
whitelist of allowed Lead fields (reject or return an error from buildSoqlQuery
if any entry is invalid), validate/normalize lastSyncTimestamp to a strict
ISO8601/SOQL datetime format before embedding, and treat soqlFilter as raw SOQL
only after explicit opt-in (document it) or restrict its allowed
patterns/characters; update buildSoqlQuery to perform these checks (use a
constant VALID_LEAD_FIELDS array, validate entries with indexOf, validate
timestamp parsing, and only append soqlFilter when it passes validation) and
return an error when validation fails so callers don’t run a constructed unsafe
query.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/README.md`:
- Around line 1-7: The README's title and description incorrectly mention
"Salesforce Opportunities" while the integration actually syncs Salesforce
Leads; update the top-level title and the Description section to reference
"Salesforce Leads to Google Sheets" and describe that the integration extracts
Salesforce Lead records and creates a timestamped Google Sheets spreadsheet
snapshot on a configurable schedule, ensuring the header and descriptive
paragraph (the lines containing the title and the "Description" paragraph)
consistently use "Leads" instead of "Opportunities".
---
Nitpick comments:
In
`@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/config-schema.json`:
- Around line 66-68: The JSON schema currently allows any string for the config
fields; tighten it by encoding the documented constraints: add an "enum" listing
allowed values for "timeframe" (the finite set documented) and for "syncMode"
(the documented options), and constrain "lastSyncTimestamp" to an ISO‑8601
datetime using "format": "date-time" or a suitable "pattern" regex; update the
corresponding other occurrences (the properties around the 78-84 block) to match
the same enums/formats so the schema rejects invalid values at validation time.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/automation.bal`:
- Around line 38-57: The code accepts syncMode as a plain string (symbol:
syncMode) so invalid values surface late; update handling to use the SyncMode
enum (symbol: SyncMode) or validate the string against that enum early:
convert/parse syncMode into a SyncMode value (or return an error) before
computing effectiveSyncMode and before any branch (symbols: effectiveSyncMode,
isNewSpreadsheet) so invalid modes fail fast; ensure the default behavior still
maps empty/whitespace to SyncMode.APPEND and adjust later branches to compare
against the enum values (APPEND, FULL_REPLACE, UPSERT_BY_EMAIL) instead of raw
strings.
- Around line 158-159: The variable existingValues (from existingRange.values)
is declared as (int|string|decimal)[][] but your SheetRow type allows
(int|string|decimal|boolean|float)[], causing a type mismatch during row
comparisons/updates; fix it by either widening the existingValues declaration to
(int|string|decimal|boolean|float)[][] or (preferably) immediately map
existingRange.values into a normalized SheetRow[][] (create a small converter
function that converts float->decimal where needed and preserves boolean) and
use that normalized array for downstream logic in the functions that read
existingRange, existingValues, and any comparison/update code that references
SheetRow.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/config.bal`:
- Around line 49-56: Add runtime validation and inline documentation for the
configurable variables so that invalid values are caught early: validate the
configurable strings timeframe and syncMode (and timezone if present elsewhere)
against the allowed sets documented in the README (e.g., timeframe ∈ {ALL,
LAST_24_HOURS, LAST_7_DAYS, ...}; syncMode ∈ {APPEND, MERGE, REPLACE, ...}), and
either reject invalid values with a clear error or coerce to a safe default at
startup (use the variables timeframe, syncMode, lastSyncTimestamp,
enableIncrementalSync to locate the consuming initialization code), and add
brief inline comments next to each configurable declaration listing the
permitted values and behavior.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/functions.bal`:
- Around line 3-7: getCurrentTimestamp currently calls
currentCivil.second.toString() but time:Civil.second is decimal and may include
fractional seconds; truncate the fractional part of currentCivil.second (e.g.,
cast or floor to an int) before formatting so seconds are an integer (use the
symbol currentCivil.second and the function getCurrentTimestamp to locate the
code) and format that integer with padZero(2) to produce strict ISO 8601
seconds.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/README.md`:
- Around line 119-131: Remove the duplicated "Default Field Mapping" block that
appears under the "Advanced Features" section; locate the repeated code block
that lists the default fields (the same array starting with "Id", "FirstName",
...) and delete it so the README contains only the original mapping defined
earlier, then ensure the "SOQL Filtering:" heading directly follows the
"Advanced Features" content with correct markdown spacing; no other content
changes are needed.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/types.bal`:
- Around line 34-40: The SyncMode enum is unused while syncMode is declared as a
configurable string; either switch the configurable to the enum or remove the
enum. To fix, update the configurable declaration (symbol: syncMode) to use type
SyncMode with a sensible default (e.g., SyncMode.APPEND) so compile-time safety
is enforced, and update any places reading/parsing the config to expect a
SyncMode value (symbols: SyncMode, syncMode); alternatively if you prefer string
config semantics, delete the unused SyncMode enum declaration to avoid dead
code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1afbe3c5-f04f-48a3-b759-8dd7e5ca780e
📒 Files selected for processing (14)
ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/config-schema.jsonballerina-integrator/salesforce-leads-to-googlesheets/.choreo/diagram.mdballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.mdballerina-integrator/salesforce-leads-to-googlesheets/.gitignoreballerina-integrator/salesforce-leads-to-googlesheets/Ballerina.tomlballerina-integrator/salesforce-leads-to-googlesheets/README.mdballerina-integrator/salesforce-leads-to-googlesheets/agents.balballerina-integrator/salesforce-leads-to-googlesheets/automation.balballerina-integrator/salesforce-leads-to-googlesheets/config.balballerina-integrator/salesforce-leads-to-googlesheets/connections.balballerina-integrator/salesforce-leads-to-googlesheets/data_mappings.balballerina-integrator/salesforce-leads-to-googlesheets/functions.balballerina-integrator/salesforce-leads-to-googlesheets/main.balballerina-integrator/salesforce-leads-to-googlesheets/types.bal
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/salesforce-leads-to-googlesheets/README.md`:
- Around line 128-131: Update the two fenced code blocks that show the Default
Field Mapping array '["Id", "FirstName", "LastName", "Email", "Phone",
"Company", "Title", "Status", "LeadSource", "Industry", "Rating", "CreatedDate",
"LastModifiedDate"]' and the Example URL block to include explicit languages:
mark the array fence as ```json and the URL fence as ```text so markdownlint
MD040 is satisfied and rendering is improved; locate the two occurrences (one
around the JSON array and the other around the example Google Sheets URL shown
in the README) and add the corresponding language specifiers to their opening
fences.
- Around line 3-7: Update the README intro to accurately reflect the
integration's actual behavior: mention that the export can be filtered or run
incrementally (respecting configured filters/incremental sync) and that outputs
can either create a new spreadsheet or write into an existing one based on the
spreadsheetId and syncMode configuration; locate phrasing around the current
summary sentences and replace the blanket statements "exports all Lead records"
and "creates a spreadsheet ... every execution" with conditional language that
references spreadsheetId and syncMode and explains both modes (new spreadsheet
with timestamp vs append/update an existing sheet) and filtered/incremental
runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d52ec59-2a5c-4bf3-8cef-fa0b6ce65a73
📒 Files selected for processing (1)
ballerina-integrator/salesforce-leads-to-googlesheets/README.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md (1)
122-122:⚠️ Potential issue | 🟡 MinorUse a unique heading title for the Google token section.
Line 122 duplicates the heading text used at Line 41, which still triggers MD024 and creates ambiguous anchors. Rename this one to a distinct title (for example, “Step 4: Obtain Google Refresh Token”).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md` at line 122, The duplicated markdown heading "Step 4: Obtain Refresh Token" conflicts with the earlier heading at Line 41 and triggers MD024; rename this second heading to a distinct title (for example, "Step 4: Obtain Google Refresh Token") so the anchor is unique and ambiguity is avoided—update the heading text in the instructions.md file where the heading appears to the new, unique title.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md`:
- Line 122: The duplicated markdown heading "Step 4: Obtain Refresh Token"
conflicts with the earlier heading at Line 41 and triggers MD024; rename this
second heading to a distinct title (for example, "Step 4: Obtain Google Refresh
Token") so the anchor is unique and ambiguity is avoided—update the heading text
in the instructions.md file where the heading appears to the new, unique title.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a949e7d-6f3d-422c-830c-2a272db11be6
📒 Files selected for processing (1)
ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
ballerina-integrator/salesforce-leads-to-googlesheets/state.bal (1)
16-25: Discarding error details inloadLastSyncTimestampis intentional but loses diagnostic info.The
on failblock logs a generic message and returns"", but the actual error (e.g., permission denied vs. file not found) is discarded. For debugging, consider logging the error:💡 Optional enhancement
} on fail error e { - log:printInfo("No previous sync state found. Using configured lastSyncTimestamp."); + log:printInfo(string `No previous sync state found (${e.message()}). Using configured lastSyncTimestamp.`); return ""; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/state.bal` around lines 16 - 25, The on-fail handler in loadLastSyncTimestamp discards the actual io:fileReadString error; update the on fail block to capture the error (e.g., bind the failure to a variable) and include the error details in the log:printInfo (or log:printError) message when referencing STATE_FILE_PATH so diagnostics show permission/file-not-found specifics while still returning "" on failure.ballerina-integrator/salesforce-leads-to-googlesheets/functions.bal (1)
77-79: SOQL injection risk acknowledged but acceptable for deployer-controlled config.The
soqlFilteris directly concatenated into the query without sanitization. This is documented in the README as accepting raw SOQL (lines 129-137), placing responsibility on the deployer. Given this is an internal integration tool where configuration is controlled by trusted operators, this is an acceptable tradeoff.For defense-in-depth, consider adding a brief log warning when
soqlFilteris non-empty:💡 Optional enhancement
if soqlFilter != "" { + log:printDebug(string `Applying custom SOQL filter: ${soqlFilter}`); whereConditions.push(soqlFilter); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/functions.bal` around lines 77 - 79, The code concatenates soqlFilter into the SOQL where clause by calling whereConditions.push(soqlFilter) which is acceptable per README but should emit a defense-in-depth warning; update the block that checks soqlFilter (the branch where soqlFilter != "" and whereConditions.push(soqlFilter)) to log a concise warning including the filter string and context (e.g., "Using raw SOQL filter from config") via the existing logger (or Ballerina logging facility) before pushing the filter so operators see that raw SOQL is being used.
🤖 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/salesforce-leads-to-googlesheets/automation.bal`:
- Around line 227-228: The clearRange call uses a hardcoded "A:Z" which doesn't
match the dynamic column range computed earlier and can leave columns uncleared;
update the sheetsClient->clearRange call to use the same dynamic A1 range
variable (the one derived from spreadsheetId/sheet.properties.title/allData
column count) instead of "A:Z" so it clears exactly all columns before calling
sheetsClient->appendValues; ensure you reference the same variable/name used
where the dynamic range is calculated and pass it as the a1Notation argument to
clearRange.
- Around line 163-164: The hard-coded "A:Z" in the call to
sheetsClient->getRange (used to populate sheets:Range existingRange and read
existingRange.values) limits comparison to 26 columns; update getRange to
request a sufficiently wide/dynamic A1 range based on the number of fields in
fieldMapping (or use a larger fixed range like "A:AZ") so all columns are read
when building existingValues and performing the upsert; adjust any helpers that
assume 26 columns to use the computed column count derived from fieldMapping
length.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/functions.bal`:
- Around line 35-45: currentCivil.dayOfWeek is a time:DayOfWeek enum so the
current code's int? dayOfWeek check always fails; replace the enum-to-int logic
in the LAST_WEEK branch by switching on currentCivil.dayOfWeek (or mapping each
time:DayOfWeek value to the appropriate daysToSubtract) instead of using
"dayOfWeek is int", set daysToSubtract accordingly (e.g., SUNDAY -> 6, MONDAY ->
0, TUESDAY -> 1, ...), then keep the rest of the computation (secondsToThisWeek,
time:utcAddSeconds, lastWeekStartUtc, thisWeekStartUtc, and the CreatedDate
return) unchanged.
---
Nitpick comments:
In `@ballerina-integrator/salesforce-leads-to-googlesheets/functions.bal`:
- Around line 77-79: The code concatenates soqlFilter into the SOQL where clause
by calling whereConditions.push(soqlFilter) which is acceptable per README but
should emit a defense-in-depth warning; update the block that checks soqlFilter
(the branch where soqlFilter != "" and whereConditions.push(soqlFilter)) to log
a concise warning including the filter string and context (e.g., "Using raw SOQL
filter from config") via the existing logger (or Ballerina logging facility)
before pushing the filter so operators see that raw SOQL is being used.
In `@ballerina-integrator/salesforce-leads-to-googlesheets/state.bal`:
- Around line 16-25: The on-fail handler in loadLastSyncTimestamp discards the
actual io:fileReadString error; update the on fail block to capture the error
(e.g., bind the failure to a variable) and include the error details in the
log:printInfo (or log:printError) message when referencing STATE_FILE_PATH so
diagnostics show permission/file-not-found specifics while still returning "" on
failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27c5a257-678d-4114-aba1-99934ac525eb
📒 Files selected for processing (6)
ballerina-integrator/salesforce-leads-to-googlesheets/README.mdballerina-integrator/salesforce-leads-to-googlesheets/agents.balballerina-integrator/salesforce-leads-to-googlesheets/automation.balballerina-integrator/salesforce-leads-to-googlesheets/data_mappings.balballerina-integrator/salesforce-leads-to-googlesheets/functions.balballerina-integrator/salesforce-leads-to-googlesheets/state.bal
|
Prebuilt Integration Checklist
|
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md (1)
62-63:⚠️ Potential issue | 🟡 MinorDocument the default value for
tabName.According to past review comments,
tabNamedefaults to "Leads". Include this information in the documentation.📝 Suggested fix
4. `tabName` + - **Default**: `"Leads"` - Base worksheet name for the export.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md` around lines 62 - 63, The doc for the `tabName` parameter is missing its default value; update the documentation in the instructions.md where `tabName` is described to state that its default is "Leads" (e.g., change the bullet to read something like "Base worksheet name for the export. Defaults to 'Leads'.") so users know the implicit worksheet name used when `tabName` is not provided.
🧹 Nitpick comments (2)
ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md (2)
57-60: Warn users about the auto-creation behavior.Creating a new spreadsheet on every run when
spreadsheetIdis empty can lead to spreadsheet proliferation and user confusion. Add a note recommending that users provide aspreadsheetIdfor production use.📝 Suggested documentation enhancement
3. `spreadsheetId` - Existing spreadsheet ID to write into. - You can extract it from `https://docs.google.com/spreadsheets/d/<spreadsheetId>/edit`. - If empty, a new spreadsheet is created per run. + - **Note**: For production use, specify a `spreadsheetId` to avoid creating multiple spreadsheets on each execution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md` around lines 57 - 60, Update the documentation for the `spreadsheetId` parameter to warn users that leaving `spreadsheetId` empty causes the integration to create a new Google Sheet on every run; add a short advisory sentence after the existing bullets recommending supplying an existing `spreadsheetId` for production use to avoid spreadsheet proliferation and confusion, and optionally note that leaving it empty is only appropriate for testing or one-off runs.
68-69: Provide an example forsoqlFilter.SOQL filter syntax can be complex. Including a concrete example would help users understand the expected format.
📝 Suggested documentation enhancement
6. `soqlFilter` - Custom SOQL `WHERE` fragment (without `WHERE`) for advanced filtering. + - **Example**: `"Country = 'USA' AND AnnualRevenue > 1000000"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md` around lines 68 - 69, Add a concrete example and brief guidance for the soqlFilter field in the documentation: update the `soqlFilter` entry to show a sample SOQL fragment (e.g. Email LIKE '%@example.com' AND CreatedDate >= 2023-01-01T00:00:00Z), note that the fragment must NOT include the leading WHERE, and add a short note about quoting/escaping string literals and date formats expected by Salesforce (ISO 8601) so users know how to construct valid filters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/projects.json:
- Around line 11-12: The project path in projects.json
("salesforce-leads-to-googlesheets") does not match the package.name in
Ballerina.toml ("salesforce_leads_to_googlesheets"); update one to match the
other to restore CI/build mapping: either change the Ballerina.toml
[package].name to "salesforce-leads-to-googlesheets" or modify the projects.json
path entry to "salesforce_leads_to_googlesheets" so the directory name,
projects.json path string, and Ballerina.toml package.name are consistent.
In
`@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md`:
- Around line 74-75: Document the default behavior for the includeConverted
parameter by explicitly stating its default value (true or false) and what that
default means for returned results; update the instructions entry for
`includeConverted` to read something like "Set true to include converted leads
in results. Default: false (converted leads are excluded)" or the correct
default used by the implementation so readers know whether converted leads are
included when the parameter is omitted.
- Around line 50-51: Update the documentation for the `timezone` field to
explicitly state the default value and behavior when omitted: specify that
`timezone` defaults to "UTC" (or whichever default the code uses), explain that
timestamps will be formatted in that timezone if no value is provided, and
include a short example (e.g., `timezone: "UTC"`) so users know the fallback and
expected format; ensure the `timezone` token in the instructions is the one
updated.
- Around line 53-55: The documentation for the configuration parameter
`timeframe` currently lists allowed values but omits the default; update the
instructions.md entry for the `timeframe` option to state the default value that
will be used when the config is not provided (e.g., "DEFAULT: ALL" or whatever
the implementation uses), and ensure the wording matches the parameter name
`timeframe` and existing bullet style so readers know which timeframe is assumed
when unset.
- Around line 65-66: The docs currently mention `fieldMapping` but don't state
whether it's required or has defaults; update the `fieldMapping` documentation
to explicitly say if it is required or optional, and if optional provide the
default set of Salesforce Lead fields used (e.g., LeadId, FirstName, LastName,
Company, Email) and their order; if required, list the mandatory field names and
expected format (ordered array of field names) and give a short example of a
minimal valid `fieldMapping` value so integrators know the minimum required
fields and their order.
- Around line 83-84: Update the description for the enableAutoFormat flag to
explicitly list the sheet formatting actions it toggles: mention that it applies
header-row bolding and background, freezes the top header row (freeze panes),
auto-sizes columns to content (column auto-fit), applies basic number/date
formatting for detected types, and can apply alternating/banded row styling;
reference the enableAutoFormat setting in the instructions section and replace
the vague phrase "automatic sheet formatting behaviors" with this concrete list
so users know exactly what is enabled.
- Around line 71-72: Document that the syncMode field defaults to "APPEND" when
not provided: update the `syncMode` bullet in the instructions to include the
default (e.g., "- Output write strategy: `APPEND`, `FULL_REPLACE`,
`UPSERT_BY_EMAIL`. Default: `APPEND`.") so readers know which mode is applied if
`syncMode` is omitted.
- Around line 77-78: Update the documentation for the enableIncrementalSync and
lastSyncTimestamp settings to clearly state the timestamp format (use ISO 8601
UTC, e.g. 2023-05-01T15:30:00Z), describe that lastSyncTimestamp is
automatically updated by the sync job after a successful run (the sync process
writes the most recent fetched lead's modifiedDate back to lastSyncTimestamp),
and explain initial-sync behavior: if enableIncrementalSync is true but
lastSyncTimestamp is empty the connector performs a full backfill (fetches all
leads) and then sets lastSyncTimestamp to the latest modifiedDate returned;
reference the exact config keys enableIncrementalSync and lastSyncTimestamp in
the text so readers can locate them.
- Around line 80-81: Update the documentation for the splitBy option to state
its default value and behavior: document that splitBy defaults to an empty
string/empty value (no splitting) and when empty the integration writes all rows
to the primary sheet rather than creating separate sheets; mention expected
examples (e.g., splitBy: "Status" creates separate sheets per Status, but
splitBy: "" or unset results in no split). Ensure the paragraph referencing
splitBy is added or replaces the current bullet so readers understand the
default and the empty-case behavior.
---
Duplicate comments:
In
`@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md`:
- Around line 62-63: The doc for the `tabName` parameter is missing its default
value; update the documentation in the instructions.md where `tabName` is
described to state that its default is "Leads" (e.g., change the bullet to read
something like "Base worksheet name for the export. Defaults to 'Leads'.") so
users know the implicit worksheet name used when `tabName` is not provided.
---
Nitpick comments:
In
`@ballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.md`:
- Around line 57-60: Update the documentation for the `spreadsheetId` parameter
to warn users that leaving `spreadsheetId` empty causes the integration to
create a new Google Sheet on every run; add a short advisory sentence after the
existing bullets recommending supplying an existing `spreadsheetId` for
production use to avoid spreadsheet proliferation and confusion, and optionally
note that leaving it empty is only appropriate for testing or one-off runs.
- Around line 68-69: Add a concrete example and brief guidance for the
soqlFilter field in the documentation: update the `soqlFilter` entry to show a
sample SOQL fragment (e.g. Email LIKE '%@example.com' AND CreatedDate >=
2023-01-01T00:00:00Z), note that the fragment must NOT include the leading
WHERE, and add a short note about quoting/escaping string literals and date
formats expected by Salesforce (ISO 8601) so users know how to construct valid
filters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d8f7810a-daad-473d-95c8-73833e26275d
📒 Files selected for processing (4)
.github/workflows/projects.jsonballerina-integrator/salesforce-leads-to-googlesheets/.choreo/instructions.mdballerina-integrator/salesforce-leads-to-googlesheets/Ballerina.tomlballerina-integrator/salesforce-leads-to-googlesheets/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- ballerina-integrator/salesforce-leads-to-googlesheets/Ballerina.toml
- ballerina-integrator/salesforce-leads-to-googlesheets/README.md
pcnfernando
left a comment
There was a problem hiding this comment.
@minuraashen Please send a seperate PR with the suggestions addressed
Purpose
This PR adds a prebuilt integration sample that syncs Salesforce leads to Google Sheets
using WSO2 Ballerina Integrator. This addresses the need for a ready-to-use integration
sample for users who want to automatically capture and track Salesforce leads in Google Sheets.
Goals
Approach
Implemented using Ballerina Integrator with:
User Stories
in Google Sheets so that I can track and analyze leads without manual data entry.
Release Note
Added a new prebuilt integration sample: Salesforce Leads to Google Sheets —
automatically syncs new Salesforce leads into a specified Google Sheet using Ballerina Integrator.
Documentation
N/A — Sample includes a README.md with setup and configuration instructions.
Automation Tests
and Google Sheets API
Security Checks
or other secrets: yes
Samples
This PR itself is a sample — demonstrates how to:
Test Environment
Learning
Summary by CodeRabbit
New Features
Documentation
Chores