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

Add method to parse bank transactions as JSON #831

Merged
merged 1 commit into from
Feb 24, 2021
Merged

Conversation

paroga
Copy link
Member

@paroga paroga commented Feb 17, 2021

This implements parsing of the Account Information Service format as
defined in the Berlin Group Group NextGenPSD2 XS2A Framework, which
is widely used across various European banks.

This is a first step to replace the current bank import features with
a standardized JSON interface.

Copy link
Member

@wvengen wvengen left a comment

Choose a reason for hiding this comment

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

Nice :) And great you added tests.
Too bad banks in NL are lagging with this.

return nil if content.empty?
data = JSON.parse content, symbolize_names: true

booked = data.try(:fetch, :transactions, nil).try(:fetch, :booked, nil) || []
Copy link
Member

Choose a reason for hiding this comment

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

Do you agree that this use of try reduces readability?
What about:

return nil if data.empty?

booked = data.fetch(:transactions, {}).fetch(:booked, [])

Copy link
Member Author

Choose a reason for hiding this comment

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

agree


def parse_account_information_amount(value)
value && value[:amount].to_f
end
Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that this doesn't really belong in the model, but in an importer class that takes the file and updates the database.

Copy link
Member Author

Choose a reason for hiding this comment

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

moved to lib

Copy link
Member

@wvengen wvengen left a comment

Choose a reason for hiding this comment

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

Thanks for the changes.
Some small things, otherwise good to go!

end

@bank_account.balance = parse_account_information_amount(balance) || @bank_account.bank_transactions.sum(:amount)
@bank_account.import_continuation_point = booked.first.try(:fetch, :entryReference, nil)
Copy link
Member

Choose a reason for hiding this comment

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

booked.first&.fetch(:entryReference, nil)


def last_transaction_date
bank_transactions.order(date: :desc).first.try(:date)
end
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little hesitant, because it invites developers to use the method, and it can't be preloaded.
That said, it makes sense to put it here, so let's leave it.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I think we can use the &. operator, that in my view is more readable (and less lenient):
bank_transactions.order(date: :desc).first&.date

This implements parsing of the Account Information Service format as
defined in the Berlin Group Group NextGenPSD2 XS2A Framework, which
is widely used across various European banks.

This is a first step to replace the current bank import features with
a standardized JSON interface.
@paroga paroga merged commit 4752a0a into foodcoops:master Feb 24, 2021
@paroga paroga deleted the bai branch February 24, 2021 13:59
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