Skip to content
This repository has been archived by the owner on Feb 25, 2022. It is now read-only.

Set future/pending transactions to uncleared #71

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dequis
Copy link
Contributor

@dequis dequis commented Jun 11, 2019

This adds support for sending 'uncleared' in the cleared field that goes
to YNAB if the dumper passes a falsy value in that field.

For the default dumper, this checks if the transaction date is in the
future.

For the N26 dumper, this reuses the pending transaction checks, so if
skip_pending_transactions is disabled those pending transactions are
sent to YNAB, to be hopefully picked up by the transaction matching.


This works, but I don't know if the transaction matching will be good enough in practice. So I'll be running this for a couple of weeks to see how it goes.

I have no way to test the non-N26 dumpers, and test coverage of those seems weak.

This adds support for sending 'uncleared' in the cleared field that goes
to YNAB if the dumper passes a falsy value in that field.

For the default dumper, this checks if the transaction date is in the
future.

For the N26 dumper, this reuses the pending transaction checks, so if
skip_pending_transactions is disabled those pending transactions are
sent to YNAB, to be hopefully picked up by the transaction matching.
@dequis dequis force-pushed the uncleared-transactions branch from 04ed64a to 8b07e45 Compare June 11, 2019 01:27
@gitviola
Copy link
Owner

This is amazing @dequis! Thank you very much for your PRs. I just merged the other ones.

Let me know how it works for you. I use two other banks, so once you say it works for you, I could clone your fork and try it on my end. If it all goes well we can merge it. The code looks excellent to me!

@@ -43,4 +44,8 @@ def category_id(_transaction)
def normalize_iban(iban)
iban.delete(' ')
end

def cleared?(transaction)
date(transaction) < Time.now
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe here we could use Date.current because the date method will return a date object (limitation of some dumpers, most banks don't return a timestamp..) and I think it reads a bit better to see a date comparing with a date. WDYT?

end
end

context 'when skip_pending_transactions feature is disabled' do
context 'when skip_pending_transactions feature is enabled' do
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thank you

@dequis
Copy link
Contributor Author

dequis commented Jun 13, 2019

Welp, the ynab-side matching isn't doing its thing.

scrot_gimp

Some of them are wildly different yeah, but the flixbus one is the same except (i guess) the exact timestamp.

This does have the advantage of leaving duplicate transactions as uncleared, so it's easy to tell which to keep.

Aaaaand after writing this I just noticed this one in the docs:

If import_id is omitted or specified as null, the transaction will be treated as a “user-entered” transaction. As such, it will be eligible to be matched against transactions later being imported (via DI, FBI, or API).

Which.. sounds easy enough, but will probably just get me uncleared ones several times.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants