Add Flatex - #305
Conversation
WalkthroughAdds Flatex broker support: new FlatexConverter (semicolon-delimited CSV parser and mapper) and FlatexRecord model; registers the Flatex CSV header in src/watcher.ts; wires the converter into the factory in src/converter.ts; updates README to list Flatex; increments GitVersion.yml next-version to 0.35.0. The converter parses and normalizes fields, looks up securities via SecurityService, maps transactions to Ghostfolio activities, reports parsing/runtime errors, and produces a GhostfolioExport object. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 3
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)
8-8:⚠️ Potential issue | 🟡 MinorStale broker count.
The text says "26 brokers" but with Flatex added, this should be "27 brokers."
🤖 Fix all issues with AI agents
In `@src/converters/flatexConverter.ts`:
- Around line 32-41: The cast logic in flatexConverter.ts (inside the function
handling context.column === "transactionInformation") only maps
"storno"→"deposit" and "kauf"→"buy" and thus misses dividends, mis-classifies
"verkauf", and leaves unknown types unmapped; update this block to (1)
explicitly check for "verkauf" before "kauf" and map it to "sell", (2) add
mappings for common dividend keywords like "dividende" and "ertragsgutschrift"
to a "dividend" (or the appropriate Ghostfolio type) and/or ensure
isIgnoredRecord filters dividends if they should be skipped, and (3) add a safe
fallback/path that returns a default value (e.g., "unknown" or null) and ensure
the downstream lookup into GhostfolioOrderType (the code that uses
record.transactionInformation) gracefully handles undefined keys by substituting
a default or throwing a clear error; reference the transactionInformation cast
block and the place where GhostfolioOrderType[...] is used to implement these
changes.
- Line 124: Import and apply dayjs's customParseFormat plugin at the top of
src/converters/flatexConverter.ts (same pattern as in
xtbConverter/tradeRepublicConverter) so dayjs can parse German-style dates;
specifically add the import for the plugin and call
dayjs.extend(customParseFormat) before any use of dayjs (ensuring the extension
occurs prior to the line using dayjs(record.bookingDate, "DD.MM.YYYY")).
In `@src/watcher.ts`:
- Around line 120-121: The headers mapping is reversed: the German 13-column
header matches FlatexConverter.processHeaders() while the English 10-column
header matches Finpension's dynamic/camelized columns; update the headers.set
calls so the German header is mapped to "flatex" and the English header to
"finpension" (or alternatively update FlatexConverter.processHeaders() to accept
the 10-column English form and Finpension parsing to accept the 13-column German
form) — locate the two headers.set(...) calls and swap their provider strings or
adjust the converter expectations accordingly.
🧹 Nitpick comments (3)
README.md (1)
114-117: Placeholder documentation for Flatex export instructions.The Flatex section only contains "to come." Since the PR checklist also notes README changes aren't finalized, please ensure actual export instructions are added before merging—or open a follow-up issue to track this.
Would you like me to open a GitHub issue to track adding the Flatex export instructions?
src/converters/flatexConverter.ts (2)
136-136: Redundant double-wrapping of dayjs object.
dateon line 124 is already a dayjs object. Wrapping it again withdayjs(date)is unnecessary.Suggested simplification
- date: dayjs(date).format("YYYY-MM-DDTHH:mm:ssZ"), + date: date.format("YYYY-MM-DDTHH:mm:ssZ"),
184-188:"withdraw"filter may be ineffective for German Flatex exports.After the
castfunction, only"storno"→"deposit"and"kauf"/"verkauf"→"buy"mappings exist. German withdrawal terms (e.g., "Auszahlung", "Abbuchung") are never mapped to"withdraw", so this filter entry won't match anything unless the raw CSV already contains the English word "withdraw."Consider either removing
"withdraw"or adding the relevant German terms to thecastfunction's mapping.
| if (context.column === "transactionInformation") { | ||
| const action = columnValue.toLocaleLowerCase(); | ||
|
|
||
| if (action.includes("storno")) { | ||
| return "deposit"; | ||
| } | ||
| else if (action.includes("kauf")) { | ||
| return "buy"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Incomplete transaction type mapping — dividends and unrecognized types are not handled.
The cast function only maps "storno" → "deposit" and "kauf" → "buy". A few concerns:
-
Dividends: Flatex CSVs commonly include dividend entries (e.g., "Dividende", "Ertragsgutschrift"). These are neither mapped nor filtered in
isIgnoredRecord, so they'll pass through with their raw German text astransactionInformation. ThenGhostfolioOrderType[record.transactionInformation](line 132) will resolve toundefined, producing an activity with no type. -
"Verkauf" handling is implicit: The
"verkauf"string does contain"kauf"as a substring, so it gets mapped to"buy"and later flipped to"sell"inon_recordbased on negative quantity. This works but is fragile—if a sell ever has a positive quantity (e.g., short covering), it would remain incorrectly categorized as"buy". At minimum, add a comment explaining this design choice; ideally, match"verkauf"explicitly before"kauf". -
No fallback for unknown types: Any unrecognized
transactionInformationvalue will produceGhostfolioOrderType[undefined_key]→undefinedon line 132.
Suggested improvement for the cast block
if (context.column === "transactionInformation") {
const action = columnValue.toLocaleLowerCase();
if (action.includes("storno")) {
return "deposit";
}
+ else if (action.includes("verkauf")) {
+ return "sell";
+ }
else if (action.includes("kauf")) {
return "buy";
}
+ else if (action.includes("dividende") || action.includes("ertrag")) {
+ return "dividend";
+ }
}🤖 Prompt for AI Agents
In `@src/converters/flatexConverter.ts` around lines 32 - 41, The cast logic in
flatexConverter.ts (inside the function handling context.column ===
"transactionInformation") only maps "storno"→"deposit" and "kauf"→"buy" and thus
misses dividends, mis-classifies "verkauf", and leaves unknown types unmapped;
update this block to (1) explicitly check for "verkauf" before "kauf" and map it
to "sell", (2) add mappings for common dividend keywords like "dividende" and
"ertragsgutschrift" to a "dividend" (or the appropriate Ghostfolio type) and/or
ensure isIgnoredRecord filters dividends if they should be skipped, and (3) add
a safe fallback/path that returns a default value (e.g., "unknown" or null) and
ensure the downstream lookup into GhostfolioOrderType (the code that uses
record.transactionInformation) gracefully handles undefined keys by substituting
a default or throwing a clear error; reference the transactionInformation cast
block and the place where GhostfolioOrderType[...] is used to implement these
changes.
| continue; | ||
| } | ||
|
|
||
| const date = dayjs(record.bookingDate, "DD.MM.YYYY"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if customParseFormat plugin is already configured anywhere in the project
echo "=== Searching for customParseFormat usage ==="
rg -n "customParseFormat" --type=ts
echo ""
echo "=== Checking dayjs imports across the codebase ==="
rg -n "import.*dayjs" --type=ts | head -20
echo ""
echo "=== Checking other converters that use dayjs with format strings ==="
rg -n 'dayjs\(.*,\s*"' --type=ts -C2
echo ""
echo "=== Checking main entry points and initialization files ==="
git ls-files | grep -E "(main|index|app|init)" | head -20Repository: dickwolff/Export-To-Ghostfolio
Length of output: 13132
🏁 Script executed:
#!/bin/bash
echo "=== Checking flatexConverter.ts imports and dayjs usage ==="
head -30 src/converters/flatexConverter.ts
echo ""
echo "=== Check if customParseFormat is mentioned anywhere in flatexConverter.ts ==="
rg -n "customParseFormat" src/converters/flatexConverter.tsRepository: dickwolff/Export-To-Ghostfolio
Length of output: 1247
flatexConverter.ts is missing the customParseFormat plugin required to parse "DD.MM.YYYY" format.
Line 124 uses dayjs(record.bookingDate, "DD.MM.YYYY"), but without the customParseFormat plugin, dayjs ignores the format parameter and falls back to native Date parsing, which will not correctly interpret German-style dates like "30.01.2024" and will produce invalid dates.
Add the plugin import and extension at the top of the file:
Fix
import dayjs from "dayjs";
+import customParseFormat from "dayjs/plugin/customParseFormat";
+dayjs.extend(customParseFormat);
import { parse } from "csv-parse";This pattern is already used consistently in other converters (xtbConverter, tradeRepublicConverter, swissquoteConverter, etc.).
🤖 Prompt for AI Agents
In `@src/converters/flatexConverter.ts` at line 124, Import and apply dayjs's
customParseFormat plugin at the top of src/converters/flatexConverter.ts (same
pattern as in xtbConverter/tradeRepublicConverter) so dayjs can parse
German-style dates; specifically add the import for the plugin and call
dayjs.extend(customParseFormat) before any use of dayjs (ensuring the extension
occurs prior to the line using dayjs(record.bookingDate, "DD.MM.YYYY")).
|
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/converters/flatexConverter.ts`:
- Around line 184-188: The isIgnoredRecord method currently checks only for
English keywords ("deposit", "withdraw") against record.transactionInformation,
but German terms like "Auszahlung" aren't mapped so withdrawals slip through;
update the check in isIgnoredRecord (and/or the cast function that produces
FlatexRecord.transactionInformation) to include German equivalents (e.g.,
"auszahlung", "einzahlung", "auszahlung") or ensure the cast mapping normalizes
German words to the English tokens used here; modify the ignoredRecordTypes
array or the casting logic around the FlatexRecord production so
transactionInformation is normalized (lowercased and translated) and the
.includes check will correctly detect withdrawals and deposits.
- Around line 49-53: The current number parsing for Flatex CSVs uses
columnValue.replace(",", ".") which only replaces the first comma and breaks
German-style numbers like "1.234,56"; update the parsing logic around the
Number.parseFloat call (the code that checks if columnValue.trim() === "" and
then returns Number.parseFloat(...)) to first strip all period thousands
separators (remove all "."), then replace the comma decimal separator with a
dot, and finally call Number.parseFloat on the cleaned string so "1.234,56"
becomes "1234.56".
🧹 Nitpick comments (2)
src/converters/flatexConverter.ts (2)
58-66: Sell detection relies solely on negative quantity — fragile if "verkauf" is mapped explicitly.This block works today only because
"verkauf"is accidentally caught by the"kauf"check. If the cast mapping is updated to return"sell"for"verkauf"directly (as recommended above), this block should be left as-is for the"storno"edge case but will no longer be needed for standard sells. Consider adding a comment explaining the intent.
124-136: Redundantdayjs()wrapper on line 136.
dateon line 124 is already a dayjs object, sodayjs(date)on line 136 is unnecessary.- const date = dayjs(record.bookingDate, "DD.MM.YYYY"); - ... - date: dayjs(date).format("YYYY-MM-DDTHH:mm:ssZ"), + const date = dayjs(record.bookingDate, "DD.MM.YYYY"); + ... + date: date.format("YYYY-MM-DDTHH:mm:ssZ"),
| if (columnValue.trim() === "") { | ||
| return 0; | ||
| } | ||
|
|
||
| return Number.parseFloat(columnValue.replace(",", ".")); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/converters/flatexConverter.ts | sed -n '40,60p'Repository: dickwolff/Export-To-Ghostfolio
Length of output: 930
🏁 Script executed:
# Check for similar number parsing patterns in other converters
fd --type=f -e ts 'onverter' src/converters/ | head -5Repository: dickwolff/Export-To-Ghostfolio
Length of output: 263
🏁 Script executed:
# Search for number parsing patterns with German locale handling
rg -n 'replace.*[","]' src/converters/ --type=ts -A2 -B2Repository: dickwolff/Export-To-Ghostfolio
Length of output: 13943
🏁 Script executed:
fd -e test.ts flatex src/converters/Repository: dickwolff/Export-To-Ghostfolio
Length of output: 56
🏁 Script executed:
cat -n src/converters/flatexConverter.test.ts | head -100Repository: dickwolff/Export-To-Ghostfolio
Length of output: 144
🏁 Script executed:
# Check if there are any sample/fixture files for Flatex
find . -name "*flatex*" -type f | grep -i sample\|fixture\|testRepository: dickwolff/Export-To-Ghostfolio
Length of output: 56
🏁 Script executed:
# Find all test files for converters
fd -e test.ts src/converters/ | head -20Repository: dickwolff/Export-To-Ghostfolio
Length of output: 439
🏁 Script executed:
# Check what Flatex data format looks like - search for test data or documentation
rg -l "flatex" --type=ts | head -10Repository: dickwolff/Export-To-Ghostfolio
Length of output: 245
🏁 Script executed:
# Look at the flatex converter in more detail, especially column definitions
cat -n src/converters/flatexConverter.ts | head -80Repository: dickwolff/Export-To-Ghostfolio
Length of output: 3452
🏁 Script executed:
# Search for documentation or comments about Flatex format
rg -i "flatex" README.md docs/ 2>/dev/null || echo "No matches found"Repository: dickwolff/Export-To-Ghostfolio
Length of output: 221
🏁 Script executed:
# Look for any sample CSV data or format documentation in the repo
find . -name "*.md" -o -name "*.txt" | xargs grep -l "flatex" 2>/dev/null || echo "No docs found"Repository: dickwolff/Export-To-Ghostfolio
Length of output: 85
🏁 Script executed:
# Test the actual behavior: what does Number.parseFloat do with multi-dot numbers?
node << 'EOF'
console.log("Test 1: '1.234,56'.replace(',', '.') =", "1.234,56".replace(",", "."));
console.log("parseFloat of above =", Number.parseFloat("1.234.56"));
console.log("");
console.log("Test 2: '123,45'.replace(',', '.') =", "123,45".replace(",", "."));
console.log("parseFloat of above =", Number.parseFloat("123.45"));
console.log("");
console.log("Test 3: Fixed approach: '1.234,56'.replace(/\./g, '').replace(',', '.') =", "1.234,56".replace(/\./g, "").replace(",", "."));
console.log("parseFloat of above =", Number.parseFloat("1234.56"));
EOFRepository: dickwolff/Export-To-Ghostfolio
Length of output: 335
Fix number parsing to handle German locale thousands separators.
String.replace(",", ".") only replaces the first comma. For German-locale Flatex exports with thousands separators (e.g., "1.234,56"), this produces "1.234.56" which parseFloat truncates to 1.234 instead of 1234.56.
Recommended fix
- return Number.parseFloat(columnValue.replace(",", "."));
+ return Number.parseFloat(columnValue.replace(/\./g, "").replace(",", "."));Remove all period thousands separators before replacing the comma decimal separator.
🤖 Prompt for AI Agents
In `@src/converters/flatexConverter.ts` around lines 49 - 53, The current number
parsing for Flatex CSVs uses columnValue.replace(",", ".") which only replaces
the first comma and breaks German-style numbers like "1.234,56"; update the
parsing logic around the Number.parseFloat call (the code that checks if
columnValue.trim() === "" and then returns Number.parseFloat(...)) to first
strip all period thousands separators (remove all "."), then replace the comma
decimal separator with a dot, and finally call Number.parseFloat on the cleaned
string so "1.234,56" becomes "1234.56".
| public isIgnoredRecord(record: FlatexRecord): boolean { | ||
| let ignoredRecordTypes = ["deposit", "withdraw"]; | ||
|
|
||
| return ignoredRecordTypes.some(t => record.transactionInformation.toLocaleLowerCase().includes(t)); | ||
| } |
There was a problem hiding this comment.
"withdraw" in ignored types will never match — German withdrawal keywords aren't mapped.
The cast function doesn't map German withdrawal terms (e.g., "Auszahlung") to "withdraw", so they pass through as raw German text. The includes("withdraw") check here will never trigger. Either add a cast mapping for withdrawals or check for the German terms directly.
🤖 Prompt for AI Agents
In `@src/converters/flatexConverter.ts` around lines 184 - 188, The
isIgnoredRecord method currently checks only for English keywords ("deposit",
"withdraw") against record.transactionInformation, but German terms like
"Auszahlung" aren't mapped so withdrawals slip through; update the check in
isIgnoredRecord (and/or the cast function that produces
FlatexRecord.transactionInformation) to include German equivalents (e.g.,
"auszahlung", "einzahlung", "auszahlung") or ensure the cast mapping normalizes
German words to the English tokens used here; modify the ignoredRecordTypes
array or the casting logic around the FlatexRecord production so
transactionInformation is normalized (lowercased and translated) and the
.includes check will correctly detect withdrawals and deposits.
1 participant
Added
Fixes
Checklist
Related issue (if applicable)
Fixes #134