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

memo field is empty #10

Closed
zdaar opened this issue Oct 4, 2022 · 23 comments · Fixed by #45
Closed

memo field is empty #10

zdaar opened this issue Oct 4, 2022 · 23 comments · Fixed by #45
Labels
enhancement New feature or request

Comments

@zdaar
Copy link

zdaar commented Oct 4, 2022

Hi,

Thanks for the great tool.
However I only get transaction amount and with no data in memo its pretty hard to make sense of it
image

Is data from my bank structured in an unexpected way ?
How can I find out and patch ?

Thanks in advance for considering this issue :)

@martinohansen
Copy link
Owner

@zdaar
Copy link
Author

zdaar commented Oct 4, 2022

Hi, thank for the reply.
I’m using the one on line 109, BNP_BE_GEBABEBB

@martinohansen
Copy link
Owner

martinohansen commented Oct 5, 2022

Looks like your bank uses the RemittanceInformationUnstructuredArray property, Ynabber doesn't handle that currently.

Can you test with this image: ghcr.io/martinohansen/ynabber:pr-11 ?

@zdaar
Copy link
Author

zdaar commented Oct 6, 2022

Hello,
Some transactions are now showing data in both payee and memo, some are still completely empty
2022-10-06_15-31-55

I dug a bit and dumped transactions from nordigen and here is what I could find in their response :

This below is a transaction that shows as empty
edit : transactions with both empty payee and memo do not have a remittanceInformationUnstructuredArray key
image

image

Payee name should be what is in creditorName, right now it seems to take the contents of remittanceInformationUnstructuredArray and stripping it of special characters and numbers, which is why when the remittanceInformationUnstructuredArray is only numbers I end up with nothing in the payee field, as shown here on this transaction which has a series of numbers as a memo and an empty payee field
image
image

Above screen also shows that the data you would want in the memo is the in the additionalInformation field.
The "creditorAccount is also not pushed, which could be useful information if appended to the payee and/or the memo field.

I also have a big problem, half of the transactions straight up never reach ynab :

2022/10/06 13:52:17 Successfully sent 314 transaction(s) to YNAB
2022/10/06 13:52:17 Run succeeded
2022/10/06 13:52:17 Waiting 5m before running again...

image

Do you think we can try and work on those issues ?
If yes, do you want me to open a separate issue for the writing problem ?

Thanks in advance for your reply and time

@martinohansen
Copy link
Owner

Maybe it's related, I'm not really sure. But it looks like we need some kind of mapping between fields from Nordigen and YNAB. Because the fields you're looking for is completely different from what I get from my bank.

It should be doable.

@martinohansen
Copy link
Owner

@zdaar so, just to be sure, do you want this mapping between Nordigen and YNAB:

YNAB Nordigen
payee "creditorName"
memo "additionalInformation" + "creditorAccount"

Is that right?

@hpernu
Copy link
Collaborator

hpernu commented Oct 15, 2022

This seems like partially the same things I am seeing i.e. payee should come from creditorName(or debitorName if the amount is positive).

I already have a patched version which does this. But there is still debate going on how to actually have this on the main image so that it stays compatible enough.

@zdaar
Copy link
Author

zdaar commented Oct 15, 2022 via email

Repository owner deleted a comment from hpernu Oct 18, 2022
@hpernu
Copy link
Collaborator

hpernu commented Oct 19, 2022

It is now possible to have payee information to come from creditorName/debtorName by setting NORDIGEN_PAYEE_SOURCE=name . Does this resolve some of your issues?

Although I see also the problem with memo field, does this payee setting make it more trackable in YNAB?

Memo field being a (completely) freeform text that YNAB does not use for anything it would be particularly tricky to get right for everyone - some may want more data, others less, and some completely write their own. Some may even want an original payment amount there if the payment is made in foreign currency converted by bank. If there was any improvement, perhaps it could be a list of possible fields concatenated together - although there are space limits on memo field as well. Whether the work to do so is sensible, compared to manual editing, I do not know.

@hpernu
Copy link
Collaborator

hpernu commented Jan 2, 2023

Can the original creator of this issue respond whether current versions bring any help here?

In particular:

  • There is a mention of some transactions being lost: does setting YNAB_IMPORT_ID_V2="2022-11-22" help?
    (Or use any date after which you want to use new less conflict prone IDs). Note: setting a date which is far in the past will reimport most recent transactions (up to the limit offered by Nordigen, typically 90 days) causing duplicate data.
  • Does setting NORDIGEN_PAYEE_SOURCE=name help in getting payee names more correct?

Memo fields may still be unrelated to what you need but less likely to be empty. Payee name should allow you to more easily correct these manually.

If there is no reply within a sensible time, should this be closed?

@zdaar
Copy link
Author

zdaar commented Jan 13, 2023

Hi, I should be able to test everything by Sunday. Thanks for your work !

@pfFredd
Copy link

pfFredd commented Feb 23, 2023

There seems to be a similar issue with "SEB Kort Bank AB Norway". For this connection, the Payee details need to come from "additionalInformation" in the Nordigen response. Not sure how it is best to solve this?

Greenshot 2023-02-23 23 29 45

@hpernu
Copy link
Collaborator

hpernu commented Feb 24, 2023

Is that the full JSON for a single transaction? i.e. There is no debtorName or creditorName ? I though they were mandatory.

Unfortunately, it looks like what Martin suggested at some point i.e. building bank specific blocks to the actual source code. This will get complicated and seems like a maintenance nightmare but possibly is the only way to handle all possible banks. As long as we get to import enough data that transactions are distinguishable, the automation for all possible cases might not be worth it. Is it enough for you to set the payees manually if the additional information ends up in memo field?

@pfFredd
Copy link

pfFredd commented Feb 24, 2023

Is that the full JSON for a single transaction? i.e. There is no debtorName or creditorName ? I though they were mandatory.

Yes, that is the full JSON for the first transaction. No debtorName or creditorName on any of the transactions in the response.

Is it enough for you to set the payees manually if the additional information ends up in memo field?
Unfortunately, missing the Payee field would prevent YNAB from automatically recognizing and categorizing future transactions so this is not really a viable solution.

Would it be possible to add an option such as NORDIGEN_PAYEE_SOURCE=custom along with another environmental variable with the field name for that bank? As we need a separate env-file for each account anyway this could probably be configured there to avoid having to add bank specific blocks to the source code it self?

@martinohansen
Copy link
Owner

Yeah, I think we need to allow the user control over the mapping between Nordigen and YNAB. I'm not sure how we should implement this yet.

@martinohansen martinohansen added the enhancement New feature or request label Feb 24, 2023
martinohansen added a commit that referenced this issue Feb 24, 2023
No bank seems to use the same Nordigen fields and the rudimentary best
effort guess on which fields to use is not gonna float in the long term.

This adds a structure for mapping every bank independently but defaults
to using the Creditor/Debtor name mapping as that seems most common.

Ref: #10
@martinohansen martinohansen linked a pull request Feb 24, 2023 that will close this issue
martinohansen added a commit that referenced this issue Feb 24, 2023
No bank seems to use the same Nordigen fields and the rudimentary best
effort guess on which fields to use is not gonna float in the long term.

This adds a structure for mapping every bank independently but defaults
to using the Creditor/Debtor name mapping as that seems most common.

BREAKING CHANGE: remove NORDIGEN_PAYEE_SOURCE config value in favor of
static mappings based on NORDIGEN_BANKID value

Ref: #10
@martinohansen
Copy link
Owner

@pfFredd if you share with me your banks ID (what you have in the NORDIGEN_BANKID environment variable) I can make a mapping for you that uses the additionalInformation field as payee.

@hpernu
Copy link
Collaborator

hpernu commented Feb 25, 2023

Would it be possible to add an option such as NORDIGEN_PAYEE_SOURCE=custom along with another environmental variable with the field name for that bank? As we need a separate env-file for each account anyway this could probably be configured there to avoid having to add bank specific blocks to the source code it self?

NORDIGEN_PAYEE_SOURCE=additionalInformatin (or perhaps just info) ? Would that work?

I still have a hard time seeiing a situation where most of the fields would ever be interesting as the payee.

We do need to do some stripping there though. At least for me, the additionalInformation is generally fairly long and contains too much information.

@pfFredd
Copy link

pfFredd commented Feb 25, 2023

@pfFredd if you share with me your banks ID (what you have in the NORDIGEN_BANKID environment variable) I can make a mapping for you that uses the additionalInformation field as payee.

@martinohansen That would be really kind of you! Thanks a lot.

The NORDIGEN_BANKID is SEB_KORT_AB_NO_SKHSFI21

Let me know if you need anything else.

@pfFredd
Copy link

pfFredd commented Feb 25, 2023

I still have a hard time seeiing a situation where most of the fields would ever be interesting as the payee.

I guess it boils down to whether there are a lot other banks using creative fields for providing the payee info, or if it is just a few.

We do need to do some stripping there though. At least for me, the additionalInformation is generally fairly long and contains too much information.

I have looked through the transaction log for SEB_KORT_AB_NO_SKHSFI21, where Payee appeads in the additionalInformation property. It seems the payee info in this field gets trimmed to 22 characters. This said it probably makes sense to do some kind of stripping as other banks may provide a lot longer strings in that property.

martinohansen added a commit that referenced this issue Feb 26, 2023
Support getting the payee from AdditionalInformation if its present in
the Nordigen transaction and no creditor/debtor nor remittance
information is available. Which is the case for SEB_KORT_AB_NO_SKHSFI21
for example.

Fixes #10
@martinohansen
Copy link
Owner

I still have a hard time seeiing a situation where most of the fields would ever be interesting as the payee.

I guess it boils down to whether there are a lot other banks using creative fields for providing the payee info, or if it is just a few.

Seems like every bank does it differently... maybe its a way for Nordigen to push their premium feature 🤷

We do need to do some stripping there though. At least for me, the additionalInformation is generally fairly long and contains too much information.

I have looked through the transaction log for SEB_KORT_AB_NO_SKHSFI21, where Payee appeads in the additionalInformation property. It seems the payee info in this field gets trimmed to 22 characters. This said it probably makes sense to do some kind of stripping as other banks may provide a lot longer strings in that property.

We already strip the payee before sending to YNAB here:

ynabber/writer/ynab/ynab.go

Lines 107 to 113 in 520a9dc

// Trim consecutive spaces from payee and truncate if too long
payee := strings.TrimSpace(space.ReplaceAllString(string(t.Payee), " "))
if len(payee) > maxPayeeSize {
log.Printf("Payee on account %s on date %s is too long - truncated to %d characters",
t.Account.Name, date, maxPayeeSize)
payee = payee[0:(maxPayeeSize - 1)]
}

@pfFredd do you mind testing with this image to see if it works for you?

$ docker pull ghcr.io/martinohansen/ynabber:pr-45

@pfFredd
Copy link

pfFredd commented Feb 26, 2023

@martinohansen Just tested and it maps Payee correctly now, thanks!

There seems to be an issue where Outflow and Inflow are swapped in YNAB, though. All purchases are listed as Inflow, and refunds appear as Outflow.

@martinohansen
Copy link
Owner

@martinohansen Just tested and it maps Payee correctly now, thanks!

There seems to be an issue where Outflow and Inflow are swapped in YNAB, though. All purchases are listed as Inflow, and refunds appear as Outflow.

Interesting 🤔

Do you mind opening another issue about it?

@pfFredd
Copy link

pfFredd commented Feb 26, 2023

@martinohansen Just tested and it maps Payee correctly now, thanks!
There seems to be an issue where Outflow and Inflow are swapped in YNAB, though. All purchases are listed as Inflow, and refunds appear as Outflow.

Interesting 🤔

Do you mind opening another issue about it?

Sure, no problem. I'll open a new issue for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants