Add support for XTB V2 cash operations export format (2025+)#327
Add support for XTB V2 cash operations export format (2025+)#327WarpPL wants to merge 11 commits into
Conversation
* Update cache folder path to prevent state sharing between parallel Jest workers. * Adjust test setup to use the new cache path.
…v2 converter support)
- Support new 4-line metadata preamble and updated column order - Auto-detect in watcher via 'Type;Ticker;Instrument' header line - Normalise XTB exchange suffixes (.UK→.L, .PL→.WA, etc.) to Yahoo format - Fold withholding tax as fee into adjacent dividend - Parse European decimal notation (comma, dot thousands separator) - Handle IKE/IKZE accounts with PLN-only cash currency - Real Yahoo Finance mock data for 3 new securities
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds support for the XTB V2+ cash-operations CSV format by introducing a new XtbConverterV2 class and XtbV2Record model, extensive Jest tests for the converter, and small integrations: registering the new converter in src/converter.ts and src/watcher.ts, updating test environment cache paths for Jest workers, and expanding README instructions to cover both new (xtb-v2) and legacy XTB export workflows. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
337-337:⚠️ Potential issue | 🟡 MinorDocument the new manual converter command.
The XTB instructions reference
xtb-v2, but the run-command table still only showsxtb, so local users may run the legacy converter by mistake.📝 Proposed table update
-| XTB | `run start xtb` | +| XTB | `run start xtb` or `run start xtb-v2` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 337, Update the README run-command table so the XTB entry reflects the new manual converter command by replacing or augmenting the current `XTB | run start xtb` entry to reference `run start xtb-v2` (or include both `xtb` and `xtb-v2` with a note). Locate the table row that uses the symbol "XTB" and change the command cell to include `run start xtb-v2` (or `run start xtb / run start xtb-v2` if preserving legacy) and add a short clarifying note indicating which is the legacy vs. new converter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 197-203: Update the README instructions for exporting the "Cash
operations" sheet to remove the contradictory "Comma-delimited" phrase:
explicitly instruct users to save as "CSV UTF-8 (Semicolon-delimited)" (or "CSV
UTF-8" and ensure the column separator is semicolon) and clarify that the
resulting CSV must use `;` as the delimiter for the xtb-v2 converter; modify the
steps around saving so step 2 uses the corrected label and step 3 simply
reiterates to set the column separator to semicolon (`;`) to avoid confusion.
In `@src/converters/xtbConverterV2.test.ts`:
- Around line 392-405: The test "should use PLN currency for all activity types
in IKE account" is a no-op and must either assert PLN behavior or be removed;
update the test that constructs XtbConverterV2 and calls readAndProcessFile to
either (a) use an input that triggers IKE detection (or call
sut.detectAccountCurrency with a filename indicating IKE) and then assert the
resulting GhostfolioExport (returned to the readAndProcessFile success callback)
has account.currency === 'PLN' and that all activities/transactions in the
export have currency === 'PLN', or (b) if you cannot simulate IKE here, delete
the test. Ensure you modify the test referencing
XtbConverterV2.readAndProcessFile and GhostfolioExport to include these explicit
assertions (or remove the test entirely).
In `@src/converters/xtbConverterV2.ts`:
- Around line 54-62: The detectAccountCurrency function currently treats any
three-letter prefix as a currency; update detectAccountCurrency (use the
existing filename and marker variables) to validate the detected three-letter
marker against a whitelist/set of valid ISO currency codes (e.g., EUR, USD, GBP,
PLN, etc.) before returning it; if marker is "IKE" or "IKZE" keep returning
"PLN", if marker exists but is not in the valid-currencies set fall back to
process.env.XTB_ACCOUNT_CURRENCY || "EUR" so arbitrary prefixes like "XTB" are
not accepted as currencies.
- Around line 238-265: When divCurrency matches the account currency, change the
dividend export to use the parsed per-share rate and derived quantity instead of
always using quantity=1 and unitPrice=abs(record.amount); specifically, after
obtaining security from securityService.getSecurity and extracting the parsed
dividend rate (from divMatch or the security record), compute quantity =
Math.abs(record.amount) / dividendRate (apply safe numeric parsing and rounding
as appropriate) and set unitPrice = dividendRate; keep the existing behavior
(quantity=1, unitPrice=abs(record.amount)) as a fallback when currencies do not
match or the dividendRate is missing, and ensure you still supply feeAmount,
comment, accountId and GhostfolioOrderType["dividend"] in the result.activities
entry.
- Around line 127-143: The dayjs parsing in XtbConverterV2 (the constructor and
the date parse at record.time) is missing the customParseFormat and utc plugins
and thus fails to parse "YYYY-MM-DD HH:mm:ss" as UTC; update the XtbConverterV2
constructor to import and extend dayjs with customParseFormat and utc, then
change the parse call to use dayjs.utc(record.time.trim(), "YYYY-MM-DD
HH:mm:ss") (or equivalent) so date handling is robust and consistent across
hosts; ensure references to dayjs.extend, the XtbConverterV2 constructor, and
the parsing site (where date = dayjs(...)) are updated accordingly.
---
Outside diff comments:
In `@README.md`:
- Line 337: Update the README run-command table so the XTB entry reflects the
new manual converter command by replacing or augmenting the current `XTB | run
start xtb` entry to reference `run start xtb-v2` (or include both `xtb` and
`xtb-v2` with a note). Locate the table row that uses the symbol "XTB" and
change the command cell to include `run start xtb-v2` (or `run start xtb / run
start xtb-v2` if preserving legacy) and add a short clarifying note indicating
which is the legacy vs. new converter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63c3252e-b41e-42c3-9493-53f3e30abf1f
⛔ Files ignored due to path filters (2)
assets/export-xtb-1.jpgis excluded by!**/*.jpgsamples/xtb-v2-export.csvis excluded by!**/*.csv
📒 Files selected for processing (11)
GitVersion.ymlREADME.mdsrc/converter.tssrc/converters/xtbConverterV2.test.tssrc/converters/xtbConverterV2.tssrc/models/xtbV2Record.tssrc/securityService.test.tssrc/testing/data/yahooFinanceQuoteSummaryResults.jsonsrc/testing/data/yahooFinanceSearchResults.jsonsrc/testing/testEnvVars.jssrc/watcher.ts
WarpPL
changed the title
|
@coderabbitai autofix |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Failed to clone repository into sandbox. Please try again. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/converters/xtbConverterV2.test.ts (1)
392-395:⚠️ Potential issue | 🟡 MinorTest name does not match what is being asserted, and duplicates the
detectAccountCurrencysuite.The
itsays "should use PLN currency for all activity types in IKE account", but the body only callsXtbConverterV2.detectAccountCurrency(...)against two filenames — both of which are already covered byreturns PLN for XTB_IKE_ filename(line 207-209) and the IKZE variants in thedetectAccountCurrencydescribe block. To honor the original intent, driveprocessFileContents(orreadAndProcessFilewith an IKE-named fixture) and assert that every activity in the resulting export hascurrency === "PLN", similar to the EUR-account test at lines 338-390. Otherwise, drop the test as redundant.🧪 Suggested rewrite to match the test name
- it("should use PLN currency for all activity types in IKE account", () => { - expect(XtbConverterV2.detectAccountCurrency("XTB_IKE_99000009_2006-01-01_2026-04-19.csv")).toBe("PLN"); - expect(XtbConverterV2.detectAccountCurrency("XTB_IKZE_99000009_2006-01-01_2026-04-19.csv")).toBe("PLN"); - }); + it("should use PLN currency for all activity types in IKE account", (done) => { + + const sut = new XtbConverterV2(new SecurityService(new YahooFinanceServiceMock())); + + let csv = ""; + csv += "Account number;99000001;;;;;;\n"; + csv += "Cash Operations;;;;;;;\n"; + csv += "Date from (UTC);2006-01-01 00:00:00;;;;;;\n"; + csv += "Date to (UTC);2026-04-19 14:07:32;;;;;;\n"; + csv += "Type;Ticker;Instrument;Time;Amount;ID;Comment;Product\n"; + csv += "Free funds interest;;;2026-04-03 16:04:18;0,02;200001;Free-funds Interest 2026-03;IKE\n"; + csv += "Stock purchase;IS0M.DE;Italy Govt Bond;2026-03-02 13:31:07;-2154,74;200003;OPEN BUY 14 @ 153.91;IKE\n"; + + // Use a filename that triggers IKE detection. + sut.processFileContents(csv, (actualExport: GhostfolioExport) => { + actualExport.activities.forEach(a => expect(a.currency).toBe("PLN")); + done(); + }, () => done.fail("Should not have an error!"), + /* fileName: */ "XTB_IKE_99000009_2006-01-01_2026-04-19.csv"); + });Note: the optional
fileNameargument above assumesprocessFileContentsaccepts one — please verify against the converter signature.#!/bin/bash # Confirm processFileContents signature so the suggested fixture-driven test compiles. ast-grep --pattern $'processFileContents($$$) { $$$ }' rg -nP -C2 '\bprocessFileContents\s*\(' --type=ts
🧹 Nitpick comments (1)
src/converters/xtbConverterV2.test.ts (1)
318-318: Avoid(sut as any)to reach a private member; expose a seam instead.
jest.spyOn((sut as any).progress, "log")reaches into a private field and silently couples the test to the internal property name. Renamingprogressin the converter would break this test at runtime with no compile-time signal. Consider one of:
- Spying on the underlying
console/logger thatprogressultimately writes to.- Adding a protected getter or a constructor-injectable progress logger so tests can assert against it through the public API.
Not blocking, but it’s a brittle seam in an otherwise clean test file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/converters/xtbConverterV2.test.ts` at line 318, The test currently reaches into a private field with jest.spyOn((sut as any).progress, "log"), which couples tests to an internal name; instead modify the converter to expose a seam and change the test to spy through that seam: either (A) add a constructor parameter to inject a progress logger (e.g., accept a ProgressLogger in the converter constructor) and in tests pass a mock/spied logger and use jest.spyOn(mockLogger, "log"), or (B) add a protected getter like getProgress() that returns the progress logger and have the test call jest.spyOn(sut.getProgress(), "log") (or subclass in tests to override it); update the test to remove the (sut as any) cast and spy on the injected/getter-provided logger so the test no longer depends on the private progress property name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/converters/xtbConverterV2.test.ts`:
- Around line 216-230: The test name for the spec using
XtbConverterV2.readAndProcessFile is misleading: it says "should use PLN
currency for INTEREST when reading IKE file" but uses
"samples/xtb-v2-export.csv" and asserts interest.currency === "EUR"; either
rename the it(...) description to reflect the EUR-default behavior being tested,
or change the fixture/filename passed to readAndProcessFile to a sample that
triggers IKE/PLN detection so the assertion for PLN is valid; update the it(...)
title string accordingly and keep the rest of the test (interest lookup and
expectation) consistent with the chosen fixture.
---
Nitpick comments:
In `@src/converters/xtbConverterV2.test.ts`:
- Line 318: The test currently reaches into a private field with jest.spyOn((sut
as any).progress, "log"), which couples tests to an internal name; instead
modify the converter to expose a seam and change the test to spy through that
seam: either (A) add a constructor parameter to inject a progress logger (e.g.,
accept a ProgressLogger in the converter constructor) and in tests pass a
mock/spied logger and use jest.spyOn(mockLogger, "log"), or (B) add a protected
getter like getProgress() that returns the progress logger and have the test
call jest.spyOn(sut.getProgress(), "log") (or subclass in tests to override it);
update the test to remove the (sut as any) cast and spy on the
injected/getter-provided logger so the test no longer depends on the private
progress property name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 423f67cd-730f-4835-9e2d-0bd6bec7aaf7
📒 Files selected for processing (3)
README.mdsrc/converters/xtbConverterV2.test.tssrc/converters/xtbConverterV2.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/converters/xtbConverterV2.ts
|
Hi @dickwolff ! |
|
Hi @dickwolff ! |
1 participant
Add support for XTB V2 export format (2025+)
Description
This PR adds support for the new XTB V2 "Cash operations" export format.
Changes
README.md)Checklist