Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect behaviour when changing from un-split to split and vice-versa #12

Closed
wizonesolutions opened this issue Dec 1, 2019 · 11 comments
Labels
bug Something isn't working

Comments

@wizonesolutions
Copy link

wizonesolutions commented Dec 1, 2019

On https://borsboom.io/foreign-currency-accounts-for-ynab/#deleted-transactions, it says:

The YNAB API does not provide a way for apps to delete transactions. As such, if a foreign currency transaction is deleted, the corresponding difference account transaction cannot be deleted. Instead the difference amount will be changed to 0 and the memo will be set to . For all intents and purposes, this has the same end result. Feel free to delete these transactions yourself, if you wish, but they won’t hurt anything.

What I'm finding, though, is that the difference amount is not set to 0. There is still an inflow or outflow. The part about the memo is true.

When I deleted the deleted transactions, it did seem to add another adjustment transaction with the total of the deleted amounts, which I assume means everything is OK?

@borsboom
Copy link
Owner

borsboom commented Dec 1, 2019

What I'm finding, though, is that the difference amount is not set to 0. There is still an inflow or outflow. The part about the memo is true.

That definitely sounds like a bug, but I'm not able to reproduce it. In theory, the only code path that sets the memo to <DELETED> should also force the transaction amount to be 0. Any chance you could provide a screenshot that demonstrates the problem, and also the output of fca4ynab with RUST_LOG environment variable set to fca4ynab=debug after deleting a foreign transaction (feel free to scrub anything you'd rather not share, such as your list of accounts)?

When I deleted the deleted transactions, it did seem to add another adjustment transaction with the total of the deleted amounts, which I assume means everything is OK?

Probably not. In all likelihood when you deleted the foreign transaction and the difference transaction wasn't reset to 0 as it should have been, the tool would have created an adjustment transaction to compensate for that. So when you deleted the difference transaction, it has to make a new adjustment to "undo" that earlier adjustment.

@borsboom borsboom added the bug Something isn't working label Dec 1, 2019
@wizonesolutions
Copy link
Author

wizonesolutions commented Dec 1, 2019

What I meant was that I deleted the non-zero transaction tagged <DELETED> from the difference account.

If this happens again, I'll try to get a screenshot/log. It created a couple deleted difference transactions when I put in a payment for an NOK credit card, but those were properly zeroed out.

@borsboom
Copy link
Owner

borsboom commented Dec 12, 2019

I just observed this bug, although unfortunately I didn't have debug logging enabled at the time. In this case, it happened after a transaction was imported (and a difference transaction was created) and I then changed it to a split transaction (which means a new difference transaction was created for the split, and the difference transaction for the parent was "deleted").

This is the regular log that shows the difference transaction update:

  Update difference transaction:
     Account: Difference credit budget account for USD
        Date: 12/11/2019
       Payee: eBay - colourmartuk
    Category: Split (Multiple Categories)...
        Memo: <DELETED> 
      Amount: 0.00

I confirmed in the code that what is displayed for the Amount is definitely exactly the same as what is being put into the UpdateTransaction object, so I'm pretty certain that it is being ignored by the YNAB API, for some reason. I have a suspicion that this has something to do with the fact that it's trying to change the category to Split, which would be a weird edge case so I'm not too surprised if the YNAB API does something unexpected here.

@borsboom
Copy link
Owner

I've now been able to reproduce, and it does indeed seem to be a case of the YNAB API ignoring the amount change:

Note the amount is 0 in the UpdateTransaction object being sent to the YNAB API:

[2019-12-12T14:23:58Z DEBUG fca4ynab::foreign_transactions_processor] Changed transactions to save to YNAB: [
        UpdateTransaction {
            id: "61465929-8f53-4942-8142-d40116a2edbc",
            account_id: "26a7f62b-04a9-49ab-8715-1d43fb9e3a26",
            date: "2019-12-12",
            amount: 0,
            payee_id: Some(
                "7c5b8617-dff9-4ab1-8e1b-58dbd7b8ad75",
            ),
            payee_name: None,
            category_id: Some(
                "36fd73ea-d126-492c-a55d-5f107a3cbfdd",
            ),
            memo: Some(
                "<DELETED> ",
            ),
            cleared: Some(
                Uncleared,
            ),
            approved: None,
            flag_color: None,
            import_id: None,
        },
    ]

But in the response, the amount is -3190 (no longer 0). The transaction ID is the same, so we're definitely dealing with the same transaction, it's just ignored the amount I specified.

[2019-12-12T14:23:59Z DEBUG fca4ynab::foreign_transactions_processor] Response from YNAB after saving changed transactions: [
        TransactionDetail {
            id: "61465929-8f53-4942-8142-d40116a2edbc",
            date: "2019-12-12",
            amount: -3190,
            memo: Some(
                "<DELETED> ",
            ),
            cleared: Uncleared,
            approved: false,
            flag_color: None,
            account_id: "26a7f62b-04a9-49ab-8715-1d43fb9e3a26",
            payee_id: Some(
                "7c5b8617-dff9-4ab1-8e1b-58dbd7b8ad75",
            ),
            category_id: Some(
                "af4b057e-d5a2-4baa-b527-ceccb5580dee",
            ),
            transfer_account_id: None,
            transfer_transaction_id: None,
            matched_transaction_id: None,
            import_id: Some(
                "FCAY:20191212:142221672:0",
            ),
            deleted: false,
            account_name: "CC <USD DIFFERENCE>",
            payee_name: Some(
                "Foo",
            ),
            category_name: Some(
                "Electric",
            ),
            subtransactions: [],
        },
    ]

In this case, I believe we can work around this by un-setting the category in this case of a regular transaction that's turned into a split.

@wizonesolutions
Copy link
Author

Actually, this sounds right. I probably imported some inflows, then I split the transaction because I allocate some money for taxes and such.

@borsboom
Copy link
Owner

borsboom commented Dec 12, 2019

Opened pull request #15 that should fix this. With this change, deleted difference transactions will just keep their old category, which means there's no chance of it being changed to the special "split" category ID. After this change, the YNAB API correctly changes the amount to 0, as expected.

@borsboom borsboom changed the title Deleted transactions don't work the same way as documented Incorrect behaviour when changing from un-split to split and vice-versa Dec 14, 2019
@borsboom
Copy link
Owner

There also seems to be a problem when you change a split transaction back to an un-split transaction. It "deletes" the difference transaction for the split, but doesn't create a new difference transaction for the parent.

borsboom added a commit that referenced this issue Dec 14, 2019
@borsboom
Copy link
Owner

I've pushed a fix to #15 for this second problem as well.

borsboom added a commit that referenced this issue Dec 14, 2019
@wizonesolutions
Copy link
Author

wizonesolutions commented Dec 14, 2019 via email

@borsboom
Copy link
Owner

Probably not, it's a pretty uncommon edge case. If it did happen, the worst case is that a difference amount was miscategorized.

borsboom added a commit that referenced this issue Dec 15, 2019
@borsboom
Copy link
Owner

Fix released in v0.1.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants