Stop erroring if every transaction change is zero #49
Conversation
In some situations, there can be a zero-value transaction in the imported CSV files. These are usually for card checks and similar usecases. Prior to this change, the ledger tool would return the `more than one account empty` error in that case. This change updates the logic to return early if the balances sum to zero. If there is any remaining balance, then the old behaviour is preserved.
| } | ||
|
|
||
| if balance.IsZero() { | ||
| // If everything adds up, then all is well. |
There was a problem hiding this comment.
This is the key assumption - I don't think this will cause other issues but I'm happy to be corrected.
|
Hi @howeyc! I hope you don't mind this PR out of the blue. I was importing some of my account CSVs and found that the tool was tripping over some zeroed-out transactions (in this case a card check was appearing in the export). The obvious workaround was just to find and remove it but I think that we could potentially just allow a transaction to have all of its account changes be 0? |
|
LGTM I think all zeros is a little odd, but I can't see any downside to allowing it. If anything they could be placeholders, or for notes, anything really. I suppose if someone finds them useful I don't have an argument against them. I'll likely merge within a day. |
|
Thanks!
Yeah it's definitely an edge case, but if it doesn't hurt to handle it then it's worth having it to save a bit of time here and there. |
In some situations, there can be a zero-value transaction in the imported CSV files. These are usually for card checks and similar usecases. Prior to this change, the ledger tool would return the
more than one account emptyerror in that case.This change updates the logic to return early if the balances sum to zero. If there is any remaining balance, then the old behaviour is preserved.