Contributors

Hello contributors,

I think most of us can agree on:

1. It's legit OCA users/contributors who adopted OCA/bank-payment expect an easy and early migration to Odoo v18 as this is a bit the promise of the OCA when we do the usual PRs dance to make the small refactors.
2. While it may not seem urgent, a big refactor of OCA/bank-payment seems legit as well and desirable in the long run (see Alexis, Acsone and Noviat points).
   

So what about a plan like that:

1. on OCA/pank-payment we get the essential modules ported to 18.0 with refactor, so users expecting a cheap and early migration get it.
2. in the meantime, a new repo like OCA/bank-payment-next is created and get the refactor on v18.0 (with possible backports to please some users eventually, some already use that in v16 it seems)

The parties in favor of the refactor agree to assume the effort for:

1. porting a minimal set of "must have" OCA/bank-payment modules to the new design in v18.0 (like 10 modules out of the 19 modules in the 16.0 branch?)
   
2. creating a set of scripts for migrating these "must have" modules from the standard 18.0 to the new 18.0 design.
3. The script may somewhat be tested a bit like OpenUpgrade is tested: a database A is created with the standard 18.0 modules, scripts are applied and tests in the next branches are run on the migrated database.

Then we should of kind make a deal where if these first parties do this work in v18.0,

Then in version 19.0, the next design is adopted as the base and the parties not in favor of the immediate refactor, assume a part of the remaining port work for some of the other important modules.

Then it would boil down to agreeing on what are these "must have modules" people in favor of the refactor must agree port and assume a migration script for. What do you think?

On Thu, Mar 27, 2025 at 11:17 AM luc.demeyer <notifications@odoo-community.org> wrote:

What about organising a 2-3 days code sprint in the short term with the key OCA contributors for the bank-payment repo to align our ideas via face-to-face meetings.

I am worried about ending up with two forks of these modules which would weaken our combined strength in this area.

I don’t think we can afford waiting on the October code sprint to sort this one out.

I am prepared to travel to France, Spain, ... if it can help to move forward on this.

 

Regards,

Luc

 

From: Alexis de Lattre <notifications@odoo-community.org>
Sent: Thursday, 27 March 2025 11:37
To: Contributors <contributors@odoo-community.org>
Subject: Re: The future of oca/bank-payment

 

 

 

Le mer. 26 mars 2025 à 18:58, Pedro M. Baeza <notifications@odoo-community.org> a écrit :

>About the work method to bring these evolutions, I also totally agree that doing changes in small steps is preferable, but the problem is when you want to do large scale improvements, it is impossible when a version is released, as you inevitably need to break compatibility for someone, even in small ways, so improvements within an Odoo series we are quickly frozen. So the best moment is when doing a major upgrade. That is why Alexis prepared the work on 16.0 to show what he was aiming at, and now is a good moment to land his work in 18.0.

 

That's not true. He continously refuses to split the changes, and has a chaotic commit history, with no explanations, data model changes, etc, all mixed, as it happened in v9.

 

I agree with most of your critics on commit history, single PR vs 1 PR per change, etc. I'm not the "perfect contributor".

 

But I'm very surprised about your critics on the way I made the "revolution" in OCA/bank-payment v9. Do you really think I could have made the revolution in v9 with the rule "1 PR per change" ? I made so many datamodel changes in v9 that, with this rule, I would have had to make 20 PRs. Is it possible to do a datamodel revolution across 20 PRs ? When I made this work during the Sorrento code sprint, I had 7 or 8 iterations on the new datamodel before finding the "best" datamodel, which is still in use today. Could you imagine 7 or 8 datamodel iterations across 20 PRs ?

 

My opinion is : the rule "one PR per change" doesn't work when you need to make big changes. This is what happened in v9 ; this is what needs to be done now.

In my v16 "revolution" PR (https://github.com/OCA/bank-payment/pull/1174), I made 33 changes. So I would have had to make 33 PRs. Given the time and effort needed to make a 1 OCA PR, the consequence is that I wouldn't start such a big work because of the lack of time and energy. But this big cleanup/refactoring/improvements was really needed, so I decided to invest the time to do it, with the same spirit that when I made the revolution in OCA/bank-payment for v9 during the Sorrento code sprint : do the big cleanup/refactoring/improvements with little considerations for the question "will it be accepted/merge in OCA" but focus on "what's best for the quality of the code of OCA/bank-payment".

 

> About the Payment Mode, In my view it does bring value, in terms of compatibility and interoperability with standard Odoo, as well as ergonomics

 

There's no such compatibility problems, and the interoperability is already assured on the generated account.payment. This is not something new of this version.

 

> avoiding two fields and models with almost identical meaning.

 

That's the problem: they are far from being identical in meaning. The payment mode is far more advanced than the payment method line. It allows you to have a text associated that it's shown in the invoices. It allows you to dynamically have bank account numbers displayed in the invoice (both customers and your own bank accounts). It allows you to aggregate as you prefer payment methods with bank accounts, etc.

 

These features are present in my module account_payment_base_oca, cf :

https://github.com/akretion/bank-payment/blob/18-add-account_payment_base_oca/account_payment_base_oca/models/account_payment_method_line.py#L74

cf fields "report_description", "show_bank_account" and "show_bank_account_chars".

 

Imagine this case: you have a generic payment mode called "SEPA DD" with 5 bank accounts (journals) linked, but other payment modes "SEPA DD Bank 1", "SEPA DD Bank 2", etc, and for certain invoices, you change the payment mode for fixing that bank, but by default, the generic one is used and the bank is not important. You won't be able to do this with your new approach.

 

In my implementation in PR #1401, you CAN have a generic payment mode called "SEPA DD" with 5 bank accounts (journals) :

1) Odoo auto-generate 5 account.payment.method.line ; they have "selectable=False" by default.

2) you manually create a 6th account.payment.method.line with :

- selectable=True

- bank_account_link = "variable"

- variable_journal_ids : M2M that points to the 5 bank journals

- journal_id = False

(company_id is defined as related=journal_id.company_id in the account module ; account_payment_base_oca changes that to a computed field, so have company_id set to self.env.company when journal_id=False)

The native 3 M2O "Payment Mode" fields (2 on res.partner and 1 on invoices) are updated to filter on selectable=True.

 

I'm the one who introduced bank_account_link=variable on payment mode during the Sorrento code sprint on OCA/bank-payment v9. Don't count on me to remove this feature !!!

 

> About the migration effort, yes, there is some

 

There's a lot of effort, and confused users about the why of this change. There's no advantages in this change. And you are not saving tons of code as the previous refactoring did. Just 2 m2o fields...

 

The why of this change is pretty simple : adopt the native datamodel of Odoo on payment mode (and keep the great features we have in OCA/bank-payment since v9, including bank_account_link=variable). In v18, there are 3 M2O fields that point to account.payment.method.line (2 on res.partner, 1 on invoice).

 

I stay in my position of keeping current modules, which are not old, are stable and with no need of changing that.

 

Just a few code example of why I initiated this big cleanup in OCA/bank-payment v16 that I now propose in v18. Most of this dirty code has been written by me back in v5, so I have no problem to critize it:

https://github.com/OCA/bank-payment/blob/17.0/account_banking_pain_base/models/account_payment_order.py#L14  => use of safe_eval

https://github.com/OCA/bank-payment/blob/17.0/account_banking_sepa_credit_transfer/models/account_payment_order.py#L161 => passing "line.currency_id.name" as string, which is then interpreted by safe_eval... beurk !

https://github.com/OCA/bank-payment/blob/17.0/account_banking_sepa_credit_transfer/models/account_payment_order.py#L172 => ISO20022 XML generation is hard-coded to 2 decimals, so the current code doesn't support currencies that have a number of decimals different than 2. This is not a problem for SEPA credit transfer, but this is a real problem for international credit transfer that use non-SEPA ISO20022 files.

https://github.com/OCA/bank-payment/blob/17.0/account_payment_order/models/account_journal.py#L10  => this code adds 2 fields "inbound_payment_order_only" and "outbound_payment_order_only" that are used nowhere ! It's just "dead code" !

 

--

Alexis de Lattre

_______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe

_______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe

--

Raphaël Valyi

Founder and consultant

http://twitter.com/rvalyi

www.akretion.com

_______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe