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

[IMP] base_bank_from_iban: automatically create banks with schwifty #170

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

len-foss
Copy link
Contributor

Using Schwifty, we can automatically get bank information from a valid IBAN, i.e. BIC, Swift, code, and bank name.
This quick POC automatically create banks at creation/write of a bank account.

In practice the current code might be annoying if used on a DB where banks have already been created with incomplete information. I don't have have any relevant experience with that so I'd be interested to know the best approach to that issue. If used on a new DB, it should just simplify neatly the encoding process.

This could also go into an overriding module.

https://schwifty.readthedocs.io/en/latest

@pedrobaeza pedrobaeza added this to the 16.0 milestone Aug 12, 2023
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Good idea!

@pedrobaeza
Copy link
Member

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-170-by-pedrobaeza-bump-major, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit ceeac3c into OCA:16.0 Oct 31, 2023
4 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at d1728b4. Thanks a lot for contributing to OCA. ❤️

@ValentinVinagre
Copy link

ValentinVinagre commented Nov 14, 2023

@pedrobaeza @carlosdauden @len-foss @Abranes Hey,
I think that PR is merged with a wrong message in the commit. Could you change it?

@pedrobaeza
Copy link
Member

Indeed, I wasn't aware, but I'm afraid now that this can't be changed.

@ValentinVinagre
Copy link

Indeed, I wasn't aware, but I'm afraid now that this can't be changed.

the branch is protected and force cannot be used?

@pedrobaeza
Copy link
Member

Doing that will screw all the people that have already pulled this branch, forcing them to do a reset, so that's too much to do, and more having passed several weeks since the merge.

@ValentinVinagre
Copy link

And a rebert and then commit again with the correct message?

@pedrobaeza
Copy link
Member

A more convoluted history... Let's fix this in v17. At the end, the commit is linked to this PR, where there is enough information.

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

Successfully merging this pull request may close these issues.

6 participants