github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-07-16 15:14:02 -05:00
Code Issues 3.0k Packages Projects Releases 58 Wiki Activity

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

New Issue
Closed
opened 2026-06-11 19:13:44 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-06-11 19:13:44 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/6498
Author: @Copilot
Created: 12/27/2025
Status: ❌ Closed

Base: master ← Head: copilot/fix-date-fields-mapping

📝 Commits (6)

  • b59a0c9 Initial plan
  • 48fafe4 Fix field mapping to aggregate fields from all transactions
  • 10bd564 Add release notes for PR #6498
  • 4313485 Optimize getFields to iterate through transactions once
  • 567dbeb Refactor transaction parsing to use Array.reduce
  • 107118c Remove type coercion in reduce, use type parameter instead

📊 Changes

4 files changed (+245 additions, -22 deletions)

View changed files

➕ packages/desktop-client/src/components/banksync/EditSyncAccount.test.ts (+187 -0)
📝 packages/desktop-client/src/components/banksync/EditSyncAccount.tsx (+39 -13)
📝 packages/desktop-client/src/components/banksync/useBankSyncAccountSettings.ts (+13 -9)
➕ upcoming-release-notes/6498.md (+6 -0)

📄 Description

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.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/6498 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 12/27/2025 **Status:** ❌ Closed **Base:** `master` ← **Head:** `copilot/fix-date-fields-mapping` --- ### 📝 Commits (6) - [`b59a0c9`](https://github.com/actualbudget/actual/commit/b59a0c9f55938e5d9bc2977ce315bfd55aba7502) Initial plan - [`48fafe4`](https://github.com/actualbudget/actual/commit/48fafe4bd6062dfd319e8a055c9ffc5ff9e1305e) Fix field mapping to aggregate fields from all transactions - [`10bd564`](https://github.com/actualbudget/actual/commit/10bd564b648db0dfe944a8a43323cdfcf210fe10) Add release notes for PR #6498 - [`4313485`](https://github.com/actualbudget/actual/commit/431348551c9fb0a5713c75884312551ad212978d) Optimize getFields to iterate through transactions once - [`567dbeb`](https://github.com/actualbudget/actual/commit/567dbebd4c3e1411178d76976ba6532db41dfaca) Refactor transaction parsing to use Array.reduce - [`107118c`](https://github.com/actualbudget/actual/commit/107118c31c744dfa8debf9a577449ab88d0e74fa) Remove type coercion in reduce, use type parameter instead ### 📊 Changes **4 files changed** (+245 additions, -22 deletions) <details> <summary>View changed files</summary> ➕ `packages/desktop-client/src/components/banksync/EditSyncAccount.test.ts` (+187 -0) 📝 `packages/desktop-client/src/components/banksync/EditSyncAccount.tsx` (+39 -13) 📝 `packages/desktop-client/src/components/banksync/useBankSyncAccountSettings.ts` (+13 -9) ➕ `upcoming-release-notes/6498.md` (+6 -0) </details> ### 📄 Description ## 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 --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-06-11 19:13:44 -05: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
tech-support
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#121284
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?