actualbudget / actual

A local-first personal finance app
https://actualbudget.org
MIT License
13.82k stars 1.1k forks source link

[Bug]: Moving transactions between accounts can create transfers with unchangable categories #2266

Open kymckay opened 8 months ago

kymckay commented 8 months ago

Verified issue does not already exist?

Is this related to GoCardless, Simplefin or another bank-sync provider?

What happened?

Reproduction:

  1. Create transactions which are transfers to an off-budget account.
  2. Categorise them.
  3. Move the transactions back to an on-budget account (select them all then use dropdown menu to change account).

Manifestation:

Fix:

What error did you receive?

No response

Where are you hosting Actual?

Docker

What browsers are you seeing the problem on?

Firefox

Operating System

Linux

shall0pass commented 8 months ago

I tried to recreate the issue. What am I doing wrong? Transfer_OffToOn

youngcw commented 8 months ago

I was able to recreate it. It seems that since a category was set when that was allowed, that app can't tell that the category should not matter any more once the transaction is moved to a different account.

image

youngcw commented 8 months ago

One option to fix this could be to add an extra check in what counts on budget to not accidentally include transfers. The visuals show that its a transfer, but the search for categories doesn't check that.

That would probably be simpler than a full on migration

youngcw commented 8 months ago

I tried to recreate the issue. What am I doing wrong?

@shall0pass You need to make the transfer originally go to an off budget account, then change the account to an on budget one after the transaction is created

shall0pass commented 8 months ago

I tried to recreate the issue. What am I doing wrong?

@shall0pass You need to make the transfer originally go to an off budget account, then change the account to an on budget one after the transaction is created

Isn't that what I did? The House Asset is off-budget.

youngcw commented 8 months ago

@shall0pass I changed the account via the all acounts page, not the original account holder page. It may be the case that there is already some logic to clean this up that isn't getting called when modifying the account instead of the payee like you did

youngcw commented 8 months ago

I think that the place to fix the core issue lives in packages/loot-core/src/server/budget/base.ts in one of either/both the handleTransactionChange or createCategory functions.

The createCategory function looks to be what determines the spent cell value and is a long sql query. This could be modified to remove on-budget transfers from the list. The handTransactionChange function could fix the issue of not unsetting the category, but that may require a db migration to fully fix.

youngcw commented 8 months ago

Looks like if you do other changes to the broken transaction, like change the cleared status, or reconcile the account things fix themselves.

I don't know why doing a different modification would unset the category properly, but it didn't happen before.