[Bug]: GoCardless inconsistent use of creditorName and debtorName fields

[matt-fidd]

Verified issue does not already exist?

• I have searched and found no existing issue

What happened?

It appears that GoCardless are inconsistent across multiple banks with their use of creditor vs debtor name. Whilst I agree that it really is GoCardless' bug to fix, it seems that it effects multiple banks from my limited testing (and I've only got access to a few providers!).

This results in us falling back to unstructured information for Payee which normally includes dates and one-off transaction information.

I propose that we use the "correct" field first, and if it's not present fallback to the "incorrect".

The proposed change could either take place in src/app-gocardless/banks/integration-bank.js in this repository where the below could be added to the normalizeTransaction function. (I prefer this option)

+    if (transaction.creditorName && !transaction.debtorName) {
+      transaction.debtorName = transaction.creditorName;
+    } else if (transaction.debtorName && !transaction.creditorName) {
+      transaction.creditorName = transaction.debtorName;
+    }

Or could be applied in the actual repository in packages/loot-core/src/server/accounts/sync.ts in the normalizeBankSyncTransactions function.

[MatissJanis]

👋 So this basically duplicates debtorName/creditorName if only one of them is set.

I wonder if that might have any knock-on effect.. hmm.

[matt-fidd]

I can't see anywhere they're really used outside of setting the payee in actual but I can see that duplicating them over here might not be the best way to pass it back to the actual backend.

What do you reckon to changing the logic in the normalizeBankSyncTransactions function of actual/packages/loot-core/src/server/accounts/sync.ts .

Another thing we could do is the same way that dates are handled, where we check all of the various gocardless payee fields in actual-server and return the most reasonable as transaction.payeeName or something similar. The same way we do with transaction.date.

For what it's worth I've reported the instances that I've seen it occur with GoCardless but I feel it is something that is pretty easy to patch up for better UX in actual.

[MatissJanis]

IMO it's best to have all the data sanitization logic in a single place - actual-server. After some more thought: I wouldn't oppose the original proposal.

And yes - I agree - if there are simple ways to improve the UX on our end - we should definitely do so. :)

+    if (transaction.creditorName && !transaction.debtorName) {
+      transaction.debtorName = transaction.creditorName;
+    } else if (transaction.debtorName && !transaction.creditorName) {
+      transaction.creditorName = transaction.debtorName;
+    }

[matt-fidd]

Happy to put a PR in for the above, just before I do what do you think of the below proposal in the interest of keeping the sanitization logic in actual-server?

In a similar way that we handle the transaction date:

    const date =
      transaction.bookingDate ||
      transaction.bookingDateTime ||
      transaction.valueDate ||
      transaction.valueDateTime;

Should we introduce new transaction.payeeName field where we check all of the different GoCardless fields? This would move the existing payee logic out of actual into actual-server as well as give the best chance of picking up a correct payeeName.

  const payee =
    transaction.creditorName ||
    transaction.debtorName ||
    transaction.remittanceInformationUnstructured ||
    (transaction.remittanceInformationUnstructuredArray || []).join(', ') ||
    transaction.additionalInformation;

[MatissJanis]

I think that makes sense, but would you mind cutting a PR? It will be easier to review it in-context when actually seeing the proposed change as a diff.