Multiple converter fixes

[dickwolff]

Added

Fixes

• 🐛 Schwab: Add support for Cash In Lieu
• 🐛 Bux: Fix headers
• 🐛 TradeRepublic: Support dynamic headers
• 🏗️ Updated a vulnerable dependency

Checklist

• Added relevant changes to README (if applicable)
• Added relevant test(s)
• Updated the GitVersion file (if not done automatically)

Related issue (if applicable)

Fixes #287 #297 #298

[coderabbitai]

Walkthrough

Updates for release 0.34.1 across converters and models: BuxConverter expands parsed numeric fields (asset, transaction, dividend, cash amounts), treats "general corporate action" as a buy, reclassifies some buys to sells or cash-debit, broadens ignored record types, and switches quantity/price sources to assetQuantity/assetPrice. SchwabConverter now recognizes "cash in lieu" as a dividend. TradeRepublicConverter.processHeaders signature now accepts a delimiter and appends extra headers when actual columns exceed known headers. BuxRecord adds transaction/dividend fields and removes trade-focused fields. Tests updated to reflect activity count and callback signatures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Schwab: handle additional types with CUSIP #210: Modifies src/converters/schwabConverter.ts and updates SchwabConverter tests; relates to the "cash in lieu" dividend classification changes.
• Traderepublic import fix #216: Modifies TradeRepublicConverter CSV handling; relates to the updated processHeaders signature and dynamic column handling.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	❓ Inconclusive	While most changes align with fixing the three linked issues, the GitVersion bump and vulnerable dependency update lack linked issue documentation, making scope assessment unclear.	Verify that the vulnerable dependency update and GitVersion changes relate to the referenced issues or document them separately as maintenance tasks.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Multiple converter fixes' accurately describes the main change: fixes applied across multiple converter files (Schwab, Bux, TradeRepublic).
Description check	✅ Passed	The description covers all required sections with concrete details: fixes are clearly listed for each converter, tests were added, GitVersion was updated, and linked issues are referenced.
Linked Issues check	✅ Passed	The Schwab converter now recognizes 'cash in lieu' as a dividend [schwabConverter.ts], addressing issue #287's requirement to handle Cash In Lieu transactions for successful Ghostfolio import.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/fixes

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/converters/tradeRepublicConverter.ts (1)

163-184: ⚠️ Potential issue | 🟠 Major

Fix signature compatibility: make splitChar parameter optional with a default value.

The processHeaders override in TradeRepublic has a more restrictive signature than the base class AbstractConverter. The base class defines splitChar as optional with a default value (splitChar = ","), but the override makes it required. This violates the Liskov Substitution Principle — calling the method through an AbstractConverter reference with a single argument would fail for TradeRepublic.

While the current call site always passes both arguments, the signature should be:

protected processHeaders(header: string, splitChar: string = ";"): string[] {

This preserves the optional parameter pattern from the base class while setting the appropriate default for Trade Republic's semicolon delimiter.

🤖 Fix all issues with AI agents

In `@src/converters/buxConverter.ts`:
- Around line 57-67: The numeric parsing block in src/converters/buxConverter.ts
that checks context.column for fields like
"transactionAmount","assetQuantity","assetPrice","dividendGrossAmount", etc.,
calls parseFloat(columnValue) directly which yields NaN for empty strings;
update the parsing to default empty/undefined columnValue to "0" (e.g.,
parseFloat(columnValue || "0")) so empty cells produce 0 and do not corrupt
downstream calculations (refer to the same numeric fields in this if-check and
any callers that rely on those parsed values such as the unitPrice
calculations).
- Around line 78-86: The reclassification order causes cash_debit records to be
misclassified because the positive-amount buy->sell check runs before the
cash_debit check; update the logic around
record.transactionType/transferType/transactionAmount so cash_debit is handled
first or make the second check mutually exclusive (e.g., change the second if to
an else if) so that when record.transferType?.toLocaleLowerCase() ===
"cash_debit" you set record.transactionType = "cash debit" before or instead of
applying the buy->sell positive-amount transformation; ensure isIgnoredRecord
will then catch cash_debit records properly.

🧹 Nitpick comments (1)

src/converters/buxConverter.test.ts (1)

145-148: Inconsistent error-callback pattern across tests.

This test now uses done(error) while all other tests in this file still use done.fail(...) (lines 40, 52, 70, 90, 113, 160). Both work, but done(error) is arguably better since done.fail is deprecated in newer Jest versions. Consider aligning the rest of the file for consistency in a follow-up.

[github-actions]

Package	Line Rate	Branch Rate	Complexity	Health
src	99%	97%	0	✔
src.converters	98%	92%	0	✔
src.helpers	100%	100%	0	✔
Summary	98% (2192 / 2244)	92% (1419 / 1538)	0	✔

[sonarqubecloud]

Quality Gate failed

Failed conditions
16.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud