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

[16.0][FIX] account_statement_import_online_gocardless: Unique ID prefer transactionId over entryReference. #711

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

kasado
Copy link

@kasado kasado commented Aug 5, 2024

At least one bank provides not unique entryReference. Suppose transactionID is unique.

@kasado kasado closed this Aug 7, 2024
@kasado kasado reopened this Aug 7, 2024
@kasado
Copy link
Author

kasado commented Aug 7, 2024

Hi @pedrobaeza could you take a look please? Assuming you are champion for online bank import addons.

Kasparas

@pedrobaeza pedrobaeza added this to the 16.0 milestone Aug 7, 2024
@pedrobaeza
Copy link
Member

What is the rationale behind this? I haven't found any duplication using entryReference. It depends a lot on each destination bank though. For most of them in my tests, transactionReference was empty.

Anyway, there is a blocking point: if you change this now, for all the existing imported transactions, you are changing their identification (if transactionReference exists), so they will be reimported again on the next synchronization. The only way to accept this is to have a parameter (for example, at journal level), defaulting to the current behavior, but switchable to your one for deciding this order.

But again, I would like to know the reasons of the change before.

@pedrobaeza pedrobaeza changed the title [FIX] account_statement_import_online_gocardless: Unique ID prefer transactionId over entryReference. [16.0][FIX] account_statement_import_online_gocardless: Unique ID prefer transactionId over entryReference. Aug 7, 2024
@kasado
Copy link
Author

kasado commented Aug 7, 2024

@pedrobaeza ,
Thank you for quick review. The issue is with Paysera LT bank. For state TAX transfers, they put the payment code in place of a reference number. So, for any payment with the same payment code, I'm getting the same transactionReference, consequently, the same unique_import_id. That transaction is silently filtered off during import. We can put this PR on hold, I'll try to discuss with Paysera, but do not expect them to change anything, (even if they agree, that it is not proper response of transactions querry).
On the other hand it looks like transactionID is better candidate for unique_import_id than entryReference.
I don't think it is a good idea to have new parameter for this only case.

@pedrobaeza
Copy link
Member

As said, you can't change now the uniqueID, as this will reimport existing transactions. At least, a parameter should be put.

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

Successfully merging this pull request may close these issues.

2 participants