Allow setting accessValidForDays and maxHistoricalDays per bank #334
Conversation
kyrias
force-pushed
the
gocardless-per-bank-accessValidForDays-maxHistoricalDays
branch
2 times, most recently
from
687b67a to
6248e9a
Compare
|
Actually, there's something strange going on here. I just looked at the requisitions that Actual had previously created and the one from December for one of my banks already had an |
|
Ah, seems it was a bug in the nordigen module: nordigen/nordigen-node@5beefda But uh. We're supposedly using 1.3.0 but the commit that updated the version to 1.3.0 (nordigen/nordigen-node@05d261c) happened after that fix, but the fix doesn't exist in the npm 1.3.0 package? :/ |
|
Yeah, upgrading to 1.4.0 has it, so it seems like 1.3.0 on NPM doesn't at all correspond to their commits that bump the version to 1.3.0, meaning that there is no (public at least) commit of the project which corresponds to the 1.3.0 that's published on NPM. 🙃 I guess I'll update the dependency as well then. |
|
Has anyone actually reported this issue to GoCardless? I'd be interested to hear back from them (since this causes 500s on their end) before jumping to solutions on our end. |
|
@MatissJanis What issue? |
|
That issue isn't why I did this, but the issue is also misleading because the GoCardless does not return a 500 error, it's the Actual server that does. GoCardless' API returns a correct error saying exactly what's wrong with the request. |
|
Though I'd also missed that they actually serve the maximum allowed |
kyrias
force-pushed
the
gocardless-per-bank-accessValidForDays-maxHistoricalDays
branch
from
6e3b56b to
7ea4ade
Compare
This also stops taking `accessValidForDays` from the client since it's hardcoded there anyway and it's simpler to just have these per-bank values in one place. Signed-off-by: Johannes Löthberg <[email protected]>
Signed-off-by: Johannes Löthberg <[email protected]>
Contrary to the claims in the nordigen-node changelog 1.3.0 did *not* fix the missing support for passing in accessValidForDays, so we have to upgrade to 1.4.0 to actually get the fix. Signed-off-by: Johannes Löthberg <[email protected]>
kyrias
force-pushed
the
gocardless-per-bank-accessValidForDays-maxHistoricalDays
branch
from
7ea4ade to
2e0c6c7
Compare
trafico-bot
bot
added
✅ Approved
trafico-bot
bot
added
✨ Merged
kyrias
deleted the
gocardless-per-bank-accessValidForDays-maxHistoricalDays
branch
…albudget#334) * Allow setting accessValidForDays per bank This also stops taking `accessValidForDays` from the client since it's hardcoded there anyway and it's simpler to just have these per-bank values in one place. Signed-off-by: Johannes Löthberg <[email protected]> * Get the max allowed maxHistoricalDays value from the GoCardless API Signed-off-by: Johannes Löthberg <[email protected]> * Upgrade nordigen-node to 1.4.0 Contrary to the claims in the nordigen-node changelog 1.3.0 did *not* fix the missing support for passing in accessValidForDays, so we have to upgrade to 1.4.0 to actually get the fix. Signed-off-by: Johannes Löthberg <[email protected]> --------- Signed-off-by: Johannes Löthberg <[email protected]>
…albudget#334) * Allow setting accessValidForDays per bank This also stops taking `accessValidForDays` from the client since it's hardcoded there anyway and it's simpler to just have these per-bank values in one place. Signed-off-by: Johannes Löthberg <[email protected]> * Get the max allowed maxHistoricalDays value from the GoCardless API Signed-off-by: Johannes Löthberg <[email protected]> * Upgrade nordigen-node to 1.4.0 Contrary to the claims in the nordigen-node changelog 1.3.0 did *not* fix the missing support for passing in accessValidForDays, so we have to upgrade to 1.4.0 to actually get the fix. Signed-off-by: Johannes Löthberg <[email protected]> --------- Signed-off-by: Johannes Löthberg <[email protected]>
Right now this shouldn't actually change anything because the Actual will only try to fetch at most 90 days worth of transactions anyway. I'll file a PR for those changes if this is approved.
I set the values for the existing integrations based on this spreadsheet and these two documentation pages.