Purpose of refactoring subscription_oca module

[HaraldPanten]

Hello,

We were considering the following issue and we need some clarification on certain questions that have arisen from studying the needs for the subscription_oca module.

Currently, the subscription_oca module is designed and intended to work only with sales; however, we are considering the possibility of refactoring it in order to make it more versatile and usable in purchases as well.

To achieve this, we will need to do the following:

1. We will create a base module called "base_subscription_oca", for example.
2. We will refactor the subscription_oca module by extracting the base functionalities. The base will move to base_subscription_oca, while the necessary adaptations will be made in sale_subscription_oca to turn it into an extension for sales.
3. We will create an extension "purchase_subscription_oca" that will depend on the base "base_subscription_oca" and will serve as an extension for purchases.

This way, the module will be much more versatile.

We believe that the module should have been designed this way initially (more or less):

• A common base.
• Several extensions according to the business scope that needs to be covered.

We would do this in V17, as we recently performed the migration ourselves (3 weeks ago), so there will be few users using it in production. This will minimize any problems to other users that may arise.

@rousseldenis @etobella @pedrobaeza could share your opinion about this?

THX in advance!

P.S: We have considered contract module as well, but it doesn't cover the needs of our project. Subscription suits better for us.

[pedrobaeza]

Why the contract module doesn't serve? I agree is a bit more complicated, but developing an exact competing one inside OCA is not good. The path to follow is to split the main contract module into several ones, or hide features with groups, not to start from scratch and continue sophisticating the other one.

You can do contract_purchase_generation the same as contract_sale_generation exists.

[HaraldPanten]

Why the contract module doesn't serve? I agree is a bit more complicated, but developing an exact competing one inside OCA is not good. The path to follow is to split the main contract module into several ones, or hide features with groups, not to start from scratch and continue sophisticating the other one.

You can do contract_purchase_generation the same as contract_sale_generation exists.

The level of sophistication of contract module is the main reason for not using it in this project. In other projects we do use it, but it doesn't suit in this one; and we prefer not to make it more complex with extensions or customizations. Using subscription is a better decision.

On the other hand, I'm talking about improving a module that already exists in this repository (subscription_oca). Our intention is to make it better separating it in several parts (base_subscription_oca and sale_subscription_oca, for example).

I understand that having several modules for similar purposes is not the most common in OCA, but we all know that we can find similar functionallities in other situations, like:

• mis_builder_budget vs account_budget_oca
• account_credit_control vs account_invoice_overdue_reminder
• etc.

And the most of this situations exist because the original module seems to be too complex depending on the project/company you're working for.

Do you know what I mean?

THX for your feedback 😉

[pedrobaeza]

They are not similar purposes. It's the SAME purpose and they look like more and more the same as those that have decided to go to the subscription path are adding features (see #1102, #1098, #1058, #1046, #1039, #1037 ...). As said, the way to go for me is to do the same split in contract, not in subscription_oca.

[HaraldPanten]

They are not similar purposes. It's the SAME purpose and they look like more and more the same as those that have decided to go to the subscription path are adding features (see #1102, #1098, #1058, #1046, #1039, #1037 ...). As said, the way to go for me is to do the same split in contract, not in subscription_oca.

Will you accept a complete refactoring for contract module in this version (V17) and divide it in several/more simple "pieces", considering that it has been stable for almost 4 months?

[rousseldenis]

They are not similar purposes. It's the SAME purpose and they look like more and more the same as those that have decided to go to the subscription path are adding features (see #1102, #1098, #1058, #1046, #1039, #1037 ...). As said, the way to go for me is to do the same split in contract, not in subscription_oca.

I tend to agree with @pedrobaeza.

A good refactoring of contract module should be great in order to identify the functional parts that can have their existence in their own module.

But I don't know correctly subscription one. Maybe identifying or doing a good comparison of features could help the path of doing it.

We could prepare that to do it during OCA days if you are not in a hurry

[pedrobaeza]

Wow, Denis agreeing with me. Let's do a party! 😛

The main issue with contract that always arises is that "it's too complicated". Part of the guilty for that is all the successor/ancestor thing you (Acsone) added on 12.0 as part of the main module. The other "complexity" comes from the duality of defining the recurrency at line level instead of contract level. I tackled a bit that some versions ago allowing to define the recurrency at contract level and propagating them to the lines. It turns out that the previous technical architecture along with this makes a code that scares a bit (with several abstracts composing at the same time), and I tend to think that is a bit over-engineered.

All that reasons led to Domatix to propose this alternative made from scratch as a mirror of the enterprise one. And it was simpler... and very limited and now it has more features and it's getting more and more complicated. But they do the exact same thing. It's not like the examples that Harald put, where the business need is the same, but with several approaches.

For me the first thing to rip in a separate module, as only few people uses it (I don't know any, but I suppose the Acsone customer that funded the development is one of them), of all the chaining lines logic. Then, fix and maybe simplify the mixin for readability (although having some little duplications).

[HaraldPanten]

Answering to your comments:

Our business workflow would be a mixing with periodical invoicing, membership and periodification of the income (account_spread_cost_revenue). Nothing special. It's a flow known by you, Pedro, as you know how do Associations work. And our main needs for periodical invoicing are:

• Create a sale with a recurring product (service, membership).
• Once the sale is confirmed, a subscription/contract has to be created (with a template), where user can manage the typical invoicing needs: payment mode, payment terms, journal, currency, start/end dates for this contract/subscription...
• This invoices have to be created with different recurrencies (monthly, quarterly, yearly...).

After several testing, subscription_oca fits better in our flow, as contract is too complex for this project. It's as when you need to buy a car. Do you need the best car of the market? Or you just need a car that can be used for the needs that you have? And the main reasons are:

• The module is newer (in V17, I mean) so... refactoring it for V17 shouldn't be a problem.
• It's more simple, it almost does what we need. Yes, we'd improve it as well, as other users are doing.
• The contract module is too complex, and (Pedro, don't get me wrong) it's not as flexible as subscription module is. Maybe because of the big changes that you commented.

Considering this, and considering that subscription_oca is an OCA module, we just want to improve something that already exists. We are not the initial authors of the module. We are trying to improve it, considering that it's already in OCA since several older versions.

On the other hand, as Pedro said, other users are improving subscription module so... Probably we are not the only team that decided to use subscription instead of contract. I'm sure that their decisions were quite similar as ours: the current complexity of contract module.

I agree to separate the contract module in different pieces, and deprecate subscription module for next versions (V18...) but I think it's already impossible to make that huge refactoring in a stable version (V17) without creating a huge trouble to other Odoo partners that are using it in their clients.

Denis, we would love to go to the OCA Days, but I think that we're no coming this year, neither 😢

How could we solve everything with a decision that satisfies all parts?

The main objectives are:

• Simplify contract module. We can do it, dividing it in several pieces.
• Stop duplicating efforts: Contract vs Subscription.
• Make these big changes without causing big troubles to the companies that are already using contract module in V17 (we need it in V17).

Let's brainstorming 👍😂

[pedrobaeza]

As commented directly, product_contract module already has some of the features you need to add to subscription_oca, so it's better to not duplicate also that part.

Ripping out in new extra modules the renewal and successors/ancestors features will totally light the module. Also an option can be enabled for not showing the line recurrency boolean and always work in the "simple mode".

About the existing users, I hardly believe somebody is using those features, but they can be detected through migration scripts, and putting that extra modules in state to install if any value in the tables is detected (maybe they clicked by accident, hehe).

[HaraldPanten]

As commented directly, product_contract module already has some of the features you need to add to subscription_oca, so it's better to not duplicate also that part.

Ripping out in new extra modules the renewal and successors/ancestors features will totally light the module. Also an option can be enabled for not showing the line recurrency boolean and always work in the "simple mode".

About the existing users, I hardly believe somebody is using those features, but they can be detected through migration scripts, and putting that extra modules in state to install if any value in the tables is detected (maybe they clicked by accident, hehe).

Considering your mentions, I understand that we would have something like:

• contract (the base module).
• contract_renewal or contract_line_renewal (just as initial names suggestions examples): For controlling the auto-renew options at contract.line level.
• contract_xxxxx or contract_line_xxxxx: For controlling the Predecessor/Successor functionalities.

WDYT?

[sbidoul]

@sbejaoui @gva-acsone

[pedrobaeza]

Considering your mentions, I understand that we would have something like:

* `contract` (the base module).
* `contract_renewal` or `contract_line_renewal` (just as initial names suggestions examples): For controlling the auto-renew options at `contract.line` level.
* `contract_xxxxx` or `contract_line_xxxxx`: For controlling the Predecessor/Successor functionalities.

WDYT?

Yes, the last one can be one called contract_line_linking or contract_line_relation.

[sbejaoui]

Hi,

I agree with both of you on splitting the base module and extracting the renewal feature and the contract line relation. This should help with maintaining and evolving the project. As @pedrobaeza proposed, we should consider a migration script for existing projects using those features.

Additionally, I believe it's time to review the mixin implementation and simplify the abstraction level. IMO, This is currently the most complex part, which discourages developers and causes them to lose interest in the contract module.

[pedrobaeza]

Yes, there are two mixins created:

• Over for joining the common things between templates and contracts themselves.
• Other for joining recurrency things together (between header and lines)

but at the end, this makes it more confusing. I vote for removing both.

[ValentinVinagre]

Hi @pedrobaeza @sbejaoui ,
I have been reviewing the module's inheritance structure and I have come to the following conclusions:

• The mixins/abstracts created are correct.
• The inheritance is somewhat confusing.
• If mixins/abstracts are deleted, the fields/functions/etc will have to be created in other models.

Do we need to delete abstract models with the inconvenience of having to create the fields/functions/etc manually in other models?
If we remove some abstract models, future changes will be more "manual", whereas with current mixins.

I have created a diagram of the inheritances to have a visual reference:

WDYT ?

[rousseldenis]

In fact, maybe recurrence functionality could be done somehow in a separate module (e.g.: base module to define base models, security, menus). FYI, I've already externalized some functionalities that can be used by other flows about recurrence there : https://github.com/OCA/server-ux/tree/13.0/base_recurrence

[ValentinVinagre]

In fact, maybe recurrence functionality could be done somehow in a separate module (e.g.: base module to define base models, security, menus). FYI, I've already externalized some functionalities that can be used by other flows about recurrence there : https://github.com/OCA/server-ux/tree/13.0/base_recurrence

We would be doing the same thing but extending it to external modules. The base_recurrence module would have to be extended, so instead of the "contract.recurrency.basic.mixin" class we would have an extension of the "recurrence.mixin" class.
It is somewhat complex to remove mixins in this case without duplicating fields/functions/etc.

But it is a possible idea.

[rousseldenis]

But it is a possible idea.

Yes, that was just a pointer to reuse some code

[pedrobaeza]

I think contract.contract should inherit from contract.template and contract.line from contract.template.line. That way, we will reduce a lot the complexity a lot.

About contract.recurrency mixins, maybe we can reduce both classes to one, and implement directly in the classes the "advanced one", although we can have some helps methods in the mixin for these advanced things. I don't think extracting it to a module would help in its maintenance, having to do PRs in 2 separate places.