github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-03-10 20:23:07 -05:00
Code Issues 250 Packages Projects Releases 53 Wiki Activity

[PR #6498] Fix field mapping to aggregate fields from all transactions instead of single sample #6567

New Issue
Closed
opened 2026-02-28 21:30:07 -06:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-02-28 21:30:07 -06:00
Owner
Copy Link

Original Pull Request: https://github.com/actualbudget/actual/pull/6498

State: closed
Merged: No

Fix field mapping based on single sample transaction hiding available DATE fields

Plan:

  • Explore codebase and understand the issue
  • Modify getFields function to accept multiple transactions
  • Update useBankSyncAccountSettings hook to pass all transactions to getFields
  • Create unit tests for the getFields function
  • Run typecheck and lint
  • Run code review
  • Final verification
  • Optimize getFields loop structure per review feedback
  • Refactor transaction parsing to use Array.reduce
  • Remove type coercion in reduce

Issue Summary:

The field mapping UI for bank sync only showed fields from a single sample transaction. If that transaction was missing certain fields (e.g., valueDate), those fields wouldn't be available for mapping even if other transactions contained them.

Solution Implemented:

✅ Modified getFields function to accept an array of transactions instead of single transaction
✅ Optimized to iterate through transactions once (O(n) instead of O(n*m)) per review feedback
✅ Updated useBankSyncAccountSettings hook with more idiomatic Array.reduce pattern
✅ Removed type coercion in favor of reduce's generic type parameter
✅ Fields are now aggregated from all available transactions
✅ First non-undefined value is used as the example for each field
✅ Added comprehensive unit tests with 7 test cases covering edge cases and the bug scenario
✅ Removed unused exampleTransaction export
✅ All tests pass (187 tests in @actual-app/web)
✅ Typecheck passes
✅ Linting passes with no warnings
✅ All code review feedback addressed

Changes Made:

  1. EditSyncAccount.tsx: Updated getFields with optimized loop structure that iterates through transactions once
  2. useBankSyncAccountSettings.ts: Parse all transactions using Array.reduce with generic type parameter, pass them to getFields as an array
  3. EditSyncAccount.test.ts: Added 7 comprehensive unit tests including specific test for the reported bug scenario

Impact:

Users will now see all available date fields (bookingDate, valueDate, postedDate, etc.) in the field mapping UI, even if the first transaction doesn't contain all of them. This fixes the issue where valueDate was hidden for accounts with pending/card transactions as the sample transaction.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Bug]: Field mapping based on single sample transaction hides available DATE fields</issue_title>
<issue_description>### Verified issue does not already exist?

  • I have searched and found no existing issue

What happened?

I realized that bank sync field mapping derives available date fields from a single sample transaction, hiding valid fields (e.g. valueDate)

When configuring Bank Sync Settings → Field mapping, the list of available date fields (e.g. bookingDate, valueDate, postedDate) is derived from a single sample transaction only.

This causes valid fields to be missing from the UI if that particular transaction does not contain them, even though other transactions for the same account do.

How can we reproduce the issue?

In EditSyncAccount.tsx, field availability is determined by inspecting one transaction object (line 115):

export const getFields = (
  transaction: Record<string, unknown>,
): MappableFieldWithExample[] =>

Observed behavior:

Account A:

  • Sample transaction contains valueDate
  • UI shows both bookingDate and valueDate

Account B:

  • Sample transaction is a card / pending transaction
  • Only bookingDate is present
  • valueDate is not offered in field mapping

This happens even if other transactions on Account B do contain valueDate.

I think field mapping should reflect all fields that may appear in any transaction for the account, not only those present in a single example transaction.

Where are you hosting Actual?

Docker

What browsers are you seeing the problem on?

Safari

Operating System

Mac OSX</issue_description>

Comments on the Issue (you are @copilot in this section)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Summary by CodeRabbit

  • Tests

    • Added comprehensive unit tests for field extraction and aggregation across multiple bank sync transactions, covering nested fields and edge cases.

  • Bug Fixes

    • Fixed field detection in bank synchronization to analyze all available transactions instead of only the first one, improving accuracy of field mapping and enabling more complete data integration.

✏️ Tip: You can customize this high-level summary in your review settings.

**Original Pull Request:** https://github.com/actualbudget/actual/pull/6498 **State:** closed **Merged:** No --- ## Fix field mapping based on single sample transaction hiding available DATE fields ### Plan: - [x] Explore codebase and understand the issue - [x] Modify `getFields` function to accept multiple transactions - [x] Update `useBankSyncAccountSettings` hook to pass all transactions to `getFields` - [x] Create unit tests for the `getFields` function - [x] Run typecheck and lint - [x] Run code review - [x] Final verification - [x] Optimize getFields loop structure per review feedback - [x] Refactor transaction parsing to use Array.reduce - [x] Remove type coercion in reduce ### Issue Summary: The field mapping UI for bank sync only showed fields from a single sample transaction. If that transaction was missing certain fields (e.g., `valueDate`), those fields wouldn't be available for mapping even if other transactions contained them. ### Solution Implemented: ✅ Modified `getFields` function to accept an array of transactions instead of single transaction ✅ Optimized to iterate through transactions once (O(n) instead of O(n*m)) per review feedback ✅ Updated `useBankSyncAccountSettings` hook with more idiomatic Array.reduce pattern ✅ Removed type coercion in favor of reduce's generic type parameter ✅ Fields are now aggregated from all available transactions ✅ First non-undefined value is used as the example for each field ✅ Added comprehensive unit tests with 7 test cases covering edge cases and the bug scenario ✅ Removed unused `exampleTransaction` export ✅ All tests pass (187 tests in @actual-app/web) ✅ Typecheck passes ✅ Linting passes with no warnings ✅ All code review feedback addressed ### Changes Made: 1. **EditSyncAccount.tsx**: Updated `getFields` with optimized loop structure that iterates through transactions once 2. **useBankSyncAccountSettings.ts**: Parse all transactions using Array.reduce with generic type parameter, pass them to `getFields` as an array 3. **EditSyncAccount.test.ts**: Added 7 comprehensive unit tests including specific test for the reported bug scenario ### Impact: Users will now see all available date fields (bookingDate, valueDate, postedDate, etc.) in the field mapping UI, even if the first transaction doesn't contain all of them. This fixes the issue where valueDate was hidden for accounts with pending/card transactions as the sample transaction. <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[Bug]: Field mapping based on single sample transaction hides available DATE fields</issue_title> > <issue_description>### Verified issue does not already exist? > > - [x] I have searched and found no existing issue > > ### What happened? > > I realized that bank sync field mapping derives available date fields from a single sample transaction, hiding valid fields (e.g. valueDate) > > When configuring Bank Sync Settings → Field mapping, the list of available date fields (e.g. bookingDate, valueDate, postedDate) is derived from a single sample transaction only. > > This causes valid fields to be missing from the UI if that particular transaction does not contain them, even though other transactions for the same account do. > > ### How can we reproduce the issue? > > In EditSyncAccount.tsx, field availability is determined by inspecting one transaction object (line 115): > > ``` > export const getFields = ( > transaction: Record<string, unknown>, > ): MappableFieldWithExample[] => > ``` > > Observed behavior: > > **Account A:** > > - Sample transaction contains valueDate > - UI shows both bookingDate and valueDate > > **Account B:** > > - Sample transaction is a card / pending transaction > - Only bookingDate is present > - valueDate is not offered in field mapping > > This happens even if other transactions on Account B do contain valueDate. > > I think field mapping should reflect all fields that may appear in any transaction for the account, not only those present in a single example transaction. > > ### Where are you hosting Actual? > > Docker > > ### What browsers are you seeing the problem on? > > Safari > > ### Operating System > > Mac OSX</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes actualbudget/actual#6490 <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added comprehensive unit tests for field extraction and aggregation across multiple bank sync transactions, covering nested fields and edge cases. * **Bug Fixes** * Fixed field detection in bank synchronization to analyze all available transactions instead of only the first one, improving accuracy of field mapping and enabling more complete data integration. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
GiteaMirror added the pull-request label 2026-02-28 21:30:07 -06:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
AI generated
API
bank sync
budgeting
bug
can’t replicate
dependencies
docker
documentation
electron
experimental feature
feature
feedback
goal templates
good first issue
help wanted
importers
maintenance
needs info
needs testing
needs triage
needs votes
openid
payees
pull-request
Mirrored from GitHub Pull Request
 regression
reports
responsive
rules
schedules
server
✨ merged
split transactions
tech debt
theme
transaction import
transaction reconciliation
transactions
translations
upstream
user interface
✅ approved
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#6567
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?