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

[8.0][FIX] Account banking sepa credit transfer: documentation and translation updates #334

Conversation

thomaspaulb
Copy link

I installed this module and encountered a silly spelling error in Dutch, as well as was looking for half an hour for the source of a certain "Compute error". Have fixed the spelling error and added documentation to help next users on their way.

@thomaspaulb thomaspaulb changed the base branch from 10.0 to 8.0 January 23, 2017 11:55
@thomaspaulb thomaspaulb changed the title 8.0 account banking sepa credit transfer documentation and translation updates [8.0 account banking sepa credit transfer documentation and translation updates Jan 23, 2017
@thomaspaulb thomaspaulb changed the title [8.0 account banking sepa credit transfer documentation and translation updates [8.0][FIX] Account banking sepa credit transfer: documentation and translation updates Jan 23, 2017
No specific configuration.
* You need to configure a valid BIC code on the banks belonging to the
source and destination bank accounts, or the export will throw a
not-so-informative 'Compute error'.

Copy link
Contributor

Choose a reason for hiding this comment

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

If there is such an error, we should fix it at the source of the problem instead of describing it in the README.
For a SEPA credit transfer, we do need BIC on source bank account but we don't need it any more on destination bank account, even for cross-borders wire transfer... and I think it works fine in Odoo.

Copy link
Author

Choose a reason for hiding this comment

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

I think you are right.

After some investigation I have seen that from version 9.0 on, the whole of this module and the modules it depends on, have been refactored. Probably the error lies in this function (8.0 vs 9.0) and more specifically here. And I think it's already fixed from 9.0 onwards.

So I think the best solution would just be to backport the whole of it to 8.0, but that is a lot of effort. Alternatively, we could do with this README update, or a small bugfix in the 8.0 module. Let me see if I can do the latter quickly.

Copy link
Author

Choose a reason for hiding this comment

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

Well, it was not that quickly, but at least I added support for the BIC being omitted (inspired by the 9.0 code) and wrote some tests. The real effort would be to backport everything, but at least this feels better than just adding a README comment.

@thomaspaulb thomaspaulb force-pushed the 8.0-account_banking_sepa_credit_transfer-documentation-and-translation-updates branch from 6d5f6f2 to e16255c Compare May 19, 2018 16:29
@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Oct 31, 2021
@github-actions github-actions bot closed this Dec 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants