fix(qif): import transaction categories from QIF files#7315
fix(qif): import transaction categories from QIF files#7315Muhammad-Nur-Alamsyah-Anwar wants to merge 2 commits intoactualbudget:masterfrom
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hello contributor! We would love to review your PR! Before we can do that, please make sure:
We do this to reduce the TOIL the core contributor team has to go through for each PR and to allow for speedy reviews and merges. For more information, please see our Contributing Guide. |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdded an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
- Update type to include optional - Map / to in - Add release note for PR actualbudget#7315
Muhammad-Nur-Alamsyah-Anwar
force-pushed
the
fix/qif-category-not-imported
branch
from
4bf6273 to
8ab4ef2
Compare
Muhammad-Nur-Alamsyah-Anwar
changed the title
There was a problem hiding this comment.
Pull request overview
Fixes QIF import so that category/subcategory parsed from L lines is surfaced to the transaction import flow (issue #3910), and documents the change in upcoming release notes.
Changes:
- Extend QIF parsing in
parseQIFto derive a category string fromcategory+subcategory. - Add a new transaction field intended to carry imported category information.
- Add release notes entry for the bugfix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| upcoming-release-notes/7315.md | Adds release note announcing QIF category import support. |
| packages/loot-core/src/server/transactions/import/parse-file.ts | Attempts to include QIF category/subcategory on parsed transactions returned from parseFile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| payee_name: string; | ||
| imported_payee: string; | ||
| notes: string; | ||
| imported_category?: string; |
There was a problem hiding this comment.
imported_category is being added to the parsed transaction objects, but it is not a recognized field in the transactions schema. These objects are later sent back to the server (ImportTransactionsModal spreads all remaining fields into the payload), and db.insertTransaction will throw when schema validation encounters an unknown field. Suggestion: don’t introduce a new top-level field here; instead populate the existing category field with the raw category string for the import modal (it gets mapped to an id before import), or ensure any extra preview-only fields are prefixed with _ / stripped before calling transactions-import.
| imported_category?: string; | |
| category?: string; |
| imported_payee: payeeSource || (fallbackUsed ? memoSource : null), | ||
| notes: | ||
| options.importNotes && !fallbackUsed ? memoSource || null : null, | ||
| imported_category: category || null, |
There was a problem hiding this comment.
imported_category: category || null will (1) not be displayed/mapped by the import modal (it looks at transaction.category), and (2) will cause imports to fail because the client sends this field back and the server schema rejects unknown transaction fields on insert/update. Suggestion: put the raw QIF category string into category (or another schema-approved field) and avoid returning imported_category from parseQIF.
| imported_category: category || null, | |
| category: category || null, |
| const category = trans.subcategory | ||
| ? `${trans.category}:${trans.subcategory}` | ||
| : trans.category; | ||
|
|
There was a problem hiding this comment.
This change introduces new behavior (carrying QIF categories through parsing) but there’s no automated coverage added/updated in the existing import test suite (parse-file.test.ts) to assert that categories are preserved/mapped as expected. Adding a targeted QIF fixture/assertion for L lines (including Category:Subcategory) would prevent regressions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/loot-core/src/server/transactions/import/parse-file.ts`:
- Around line 187-190: The category composition currently does
`${trans.category}:${trans.subcategory}` whenever trans.subcategory exists,
which can produce invalid strings when trans.category is missing; update the
logic where the variable category is set (and the other similar occurrence at
the second composition) to only join with ':' when both trans.category and
trans.subcategory are truthy, otherwise use the single defined part
(trans.category || trans.subcategory) or a safe fallback (e.g., '' or
'Uncategorized') so imported_category never becomes something like
"undefined:Groceries".
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3aa86a5-3f9b-4302-a047-17674e6c7582
📒 Files selected for processing (2)
packages/loot-core/src/server/transactions/import/parse-file.tsupcoming-release-notes/7315.md
✅ Files skipped from review due to trivial changes (1)
- upcoming-release-notes/7315.md
packages/loot-core/src/server/transactions/import/parse-file.ts
Outdated
Show resolved
Hide resolved
- Update type to include optional - Map / to in - Add release note for PR actualbudget#7315
Muhammad-Nur-Alamsyah-Anwar
force-pushed
the
fix/qif-category-not-imported
branch
from
8ab4ef2 to
913bb01
Compare
Muhammad-Nur-Alamsyah-Anwar
force-pushed
the
fix/qif-category-not-imported
branch
from
913bb01 to
b92e831
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/loot-core/src/server/transactions/import/parse-file.ts (1)
187-190:⚠️ Potential issue | 🟡 MinorGuard category composition to avoid malformed
imported_categoryvalues.At Line 187, joining whenever
trans.subcategoryexists can emit invalid values likeundefined:Groceriesiftrans.categoryis absent. Compose with:only when both parts are present; otherwise use whichever single part exists.Proposed fix
- const category = trans.subcategory - ? `${trans.category}:${trans.subcategory}` - : trans.category; + const importedCategory = + trans.category && trans.subcategory + ? `${trans.category}:${trans.subcategory}` + : trans.category || trans.subcategory || null; return { amount: trans.amount != null ? looselyParseAmount(trans.amount) : null, date: trans.date, payee_name: payeeSource || (fallbackUsed ? memoSource : null), imported_payee: payeeSource || (fallbackUsed ? memoSource : null), notes: options.importNotes && !fallbackUsed ? memoSource || null : null, - imported_category: category || null, + imported_category: importedCategory, };Also applies to: 199-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/loot-core/src/server/transactions/import/parse-file.ts` around lines 187 - 190, The current category composition assigns `category` by concatenating `trans.category` and `trans.subcategory` when `trans.subcategory` exists, which can produce malformed values like "undefined:Groceries"; update the logic where `category` is computed (the assignment that references `trans.subcategory` and `trans.category`) to only join with ":" when both `trans.category` and `trans.subcategory` are non-empty/defined, otherwise set `category` to whichever of `trans.category` or `trans.subcategory` is present (or undefined/null if neither); apply the same guarded composition fix to the other occurrence mentioned (the similar assignment around the other block at the later occurrence).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/loot-core/src/server/transactions/import/parse-file.ts`:
- Around line 187-190: The current category composition assigns `category` by
concatenating `trans.category` and `trans.subcategory` when `trans.subcategory`
exists, which can produce malformed values like "undefined:Groceries"; update
the logic where `category` is computed (the assignment that references
`trans.subcategory` and `trans.category`) to only join with ":" when both
`trans.category` and `trans.subcategory` are non-empty/defined, otherwise set
`category` to whichever of `trans.category` or `trans.subcategory` is present
(or undefined/null if neither); apply the same guarded composition fix to the
other occurrence mentioned (the similar assignment around the other block at the
later occurrence).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 957468cb-08e3-43e1-b02d-1a8163e355e4
📒 Files selected for processing (2)
packages/loot-core/src/server/transactions/import/parse-file.tsupcoming-release-notes/7315.md
✅ Files skipped from review due to trivial changes (1)
- upcoming-release-notes/7315.md
QIF files use L lines to specify transaction categories, and qif2json already parses them into category/subcategory. The problem was in parseQIF — it was just dropping them instead of mapping them through. Also guard against "undefined:Groceries" when only subcategory is set. Fixes actualbudget#3910
|
Hey, I've addressed the review comments in the latest commits — switched to the |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me fetch the latest state of the changed files to conduct a thorough re-review. 🏁 Script executed: Length of output: 46590
✅ ✅ Category guard logic — The fix correctly handles all cases:
✅ Unit tests — Two new tests in
✅ QIF fixture ( The implementation looks solid and all previously raised concerns have been properly resolved. The PR is ready from a code review perspective. 🐇 🧠 Learnings used |
Muhammad-Nur-Alamsyah-Anwar
force-pushed
the
fix/qif-category-not-imported
branch
from
78d2b6b to
c018209
Compare
|
Tip For best results, initiate chat on the files or code changes.
All previously raised concerns have been properly resolved. The PR is ready from a code review perspective. 🐇 🧠 Learnings used |
Muhammad-Nur-Alamsyah-Anwar
force-pushed
the
fix/qif-category-not-imported
branch
from
5a5922b to
f6bd920
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
| const fallbackUsed = !payeeSource && swap; | ||
|
|
||
| const importedCategory = | ||
| trans.category && trans.subcategory |
There was a problem hiding this comment.
As an enhancement, if both category and subcategory are created, could we check if the category already exists as a category group and import to the subcategory -> category instead of joining the names?
| QIF | Actual |
|---|---|
| category | category group |
| subcategory | category |
There was a problem hiding this comment.
I agree. If the : is standard in QIF to separate category and subcategory, then they should be mapped to Actual directly as group and category.
matt-fidd
force-pushed
the
master
branch
2 times, most recently
from
5c7c70d to
d262f7d
Compare
Description
QIF files use
Llines to specify transaction categories, andqif2jsonalready parses them correctly intocategory/subcategoryfields. The problem was inparseQIF— it was just dropping those values instead of mapping them through to the returned transaction.Related issue(s)
Fixes #3910
Testing
Added a QIF fixture with a few different
Lline formats and unit tests inparse-file.test.tsto cover the main cases (category only, category + subcategory, and noLline at all). To reproduce the original bug, import a QIF file containingLlines and check that the category column is populated in the import modal.Checklist
Bundle Stats
View detailed bundle stats
desktop-client
Total
View detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged
loot-core
Total
Changeset
home/runner/work/actual/actual/packages/loot-core/src/server/transactions/import/parse-file.tsView detailed bundle breakdown
Added
Removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged
api
Total
Changeset
home/runner/work/actual/actual/packages/loot-core/src/server/transactions/import/parse-file.tsView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
cli
Total
View detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged