Skip to content

[19.0][MIG] agreement_legal: Migration to 19.0 + Refactoring - #121

Closed
CRogos wants to merge 88 commits into
OCA:19.0from
c4a8-odoo:19.0-ref-agreement_legal
Closed

[19.0][MIG] agreement_legal: Migration to 19.0 + Refactoring#121
CRogos wants to merge 88 commits into
OCA:19.0from
c4a8-odoo:19.0-ref-agreement_legal

Conversation

@CRogos

@CRogos CRogos commented Jul 8, 2026

Copy link
Copy Markdown

@tarteo, @ygol, @max3903 I mention you because you were active in #104

I would like to take the opportunity to split the agreement_legal into several modules with the migration to odoo 19.

Changes in agreement and agreement_sale are not part of this PR. There is a separate PR to set the foundations: #122

Currently I've identified the follwing modules:
agreement_legal
agreement_signature
agreement_termination
agreement_legal_content
agreement_product
agreement_revision
agreement_stage
agreement_type

(There is a screenshot in each module, that shoes the change if each module individually)
image

The goal is to keep the current functionality when all modules are installed, but make it possible to only install a subset if the functions.

This PR will replace: #120
This PR depends on: #122

@OCA-git-bot OCA-git-bot added series:19.0 mod:agreement_legal Module agreement_legal mod:agreement_legal_product Module agreement_legal_product mod:agreement_legal_stage Module agreement_legal_stage mod:agreement_legal_revision Module agreement_legal_revision mod:agreement_signature Module agreement_signature mod:agreement_legal_type Module agreement_legal_type mod:agreement_legal_content Module agreement_legal_content mod:agreement_legal_termination Module agreement_legal_termination mod:agreement_termination Module agreement_termination and removed mod:agreement_legal_termination Module agreement_legal_termination labels Jul 8, 2026
@CRogos
CRogos force-pushed the 19.0-ref-agreement_legal branch from fb71bba to de81649 Compare July 8, 2026 20:51
@max3903

max3903 commented Jul 9, 2026

Copy link
Copy Markdown
Member

@CRogos We don't need to split the module. We can use group_* settings to activate/show features.

@CRogos

CRogos commented Jul 9, 2026

Copy link
Copy Markdown
Author

@max3903 I understand your argument, and it is a valid approach. But is this the better solution? It would still blow up the database, and also the complexity of the module is still high.

With the current split it is also possible to use agreement_signature and agreement_termination without agreement_legal. My goal would be to achive this also for agreement_legal_revision, agreement_legal_stage and agreement_legal_type, but I am currently struggling with the access groups.

Do you disagree with splitting the module? (and how strong ;) )

@OCA-git-bot OCA-git-bot added mod:agreement Module agreement mod:agreement_parent Module agreement_parent labels Jul 10, 2026
@CRogos
CRogos force-pushed the 19.0-ref-agreement_legal branch from 4561579 to 3d17e51 Compare July 10, 2026 16:54
@OCA-git-bot OCA-git-bot removed the mod:agreement_parent Module agreement_parent label Jul 10, 2026
@CRogos
CRogos force-pushed the 19.0-ref-agreement_legal branch from 3d17e51 to 0639989 Compare July 10, 2026 17:47
@CRogos
CRogos marked this pull request as ready for review July 10, 2026 18:04
@CRogos
CRogos force-pushed the 19.0-ref-agreement_legal branch from 0639989 to 875197b Compare July 10, 2026 19:12
@OCA-git-bot OCA-git-bot added the mod:agreement_project Module agreement_project label Jul 10, 2026
@max3903

max3903 commented Jul 11, 2026

Copy link
Copy Markdown
Member

@CRogos Yes, I disagree with the split. Experience shows me that:

  1. if you split the module, it will age pretty badly across migrations.
  2. you may be implementing and supporting those modules now, but what about in 1+ year?

When version 20.0 will be released, those 8 modules will not get migrated or they will, but not the way you expect. Worst thing is you may not be available to review those.

Same thing will happen with version 21, 22, etc... as you may not be financed to support them anymore.

So when you will look at those modules in 3+ versions, you will not recognize them: you will think you can use them for a new project and save some money, but you will end up putting the effort again or losing the opportunity or both.

@max3903

max3903 commented Jul 11, 2026

Copy link
Copy Markdown
Member

@CRogos Also, I didn't build the OCA to be a module factory, I built it to provide long-term solutions.

@max3903 max3903 self-assigned this Jul 11, 2026
@max3903 max3903 added this to the 19.0 milestone Jul 11, 2026
@max3903

max3903 commented Jul 11, 2026

Copy link
Copy Markdown
Member

/ocabot migration agreement_legal

@OCA-git-bot OCA-git-bot mentioned this pull request Jul 11, 2026
6 tasks
@OCA-git-bot OCA-git-bot added the mod:agreement_template Module agreement_template label Aug 7, 2026
@CRogos
CRogos force-pushed the 19.0-ref-agreement_legal branch 7 times, most recently from a0f6a01 to 21f2f0a Compare August 7, 2026 12:49
@CRogos

CRogos commented Aug 7, 2026

Copy link
Copy Markdown
Author

@max3903 I've implemented it by extracting template into the module agreement_template. The template feature is still configurable by group_

@peluko00 #124 should now be integrated. Could you test the modules?

I am now retesting the migration script and before we merge I would like to squash the last two commits.

@CRogos
CRogos force-pushed the 19.0-ref-agreement_legal branch from 21f2f0a to 8a9dbcb Compare August 10, 2026 08:41
@haivanlive

Copy link
Copy Markdown

@CRogos
I can't create an agreement template, pls fix it

image

@CRogos

CRogos commented Aug 13, 2026

Copy link
Copy Markdown
Author

@haivanlive thanks. The New button is now visible again for the manager role.

@CRogos
CRogos force-pushed the 19.0-ref-agreement_legal branch 4 times, most recently from c511f9d to 392043f Compare August 18, 2026 17:19
@CRogos

CRogos commented Aug 18, 2026

Copy link
Copy Markdown
Author

@max3903 could you update your review? The template recompute issue should be solved.

The migration script worked, but during testing I still finding minor issues.

@CRogos
CRogos force-pushed the 19.0-ref-agreement_legal branch from 392043f to 7bde839 Compare August 18, 2026 18:53
@max3903

max3903 commented Aug 18, 2026

Copy link
Copy Markdown
Member

@CRogos I have stated my position on this refactoring. Don't expect me to approve it in its current state.

@CRogos

CRogos commented Aug 18, 2026

Copy link
Copy Markdown
Author

@max3903 of cause not, but you could confirm that the missing functionality is implemented now.
I have the problem, that the v18 version of agreement_legal is only usable with highly customization, which removes all the benefits of this module.

I think we should collect more feedback maybe from @peluko00 @ygol @haivanlive , or some of the agreement module users like @vangnp3702 if they also benefit from the feature separation.

@peluko00

Copy link
Copy Markdown

@max3903 of cause not, but you could confirm that the missing functionality is implemented now. I have the problem, that the v18 version of agreement_legal is only usable with highly customization, which removes all the benefits of this module.

I think we should collect more feedback maybe from @peluko00 @ygol @haivanlive , or some of the agreement module users like @vangnp3702 if they also benefit from the feature separation.

I have the same idea as @max3903 to don't split the module.
The feature for example for recompute can be used by an activate feature in config.

@ygol

ygol commented Aug 19, 2026 via email

Copy link
Copy Markdown
Contributor

@CRogos

CRogos commented Aug 19, 2026

Copy link
Copy Markdown
Author

@peluko00 I've moved recompute to the agreement_template. agreement_revision and agreement_product depends on template. Moving all the template stuff into agreement module would make things easier, but I did not want to change to much on the agreement base module. The agreement_template feature itself is still controllable by user group, as before.

I still don't see the benefit, why not to move things like revision or stage into separate modules which can be used by agreement, without agreement_legal. If you come to the OCA Days you may explain it to my on a beer.

I think the best way to continue is to split in to two worlds. I'll rename this agreement_legal into agreement_* focusing on my requirements and not on backwards compatibility. I'll mark all the modules in this PR as incompatible with "agreement_legal" for better distinction. I'll also remove agreement_legal_content because this part is ugly anyway.

Someone else can than provide a new 1:1 migration of agreement_legal according to your needs.

@vangnp3702 do you have contact to other agreement module users? Maybe we can discus what else could be added/changed to the agreement module. The settings menu and security roles have already been merged into agreement, but maybe we could also add a kanban view or move some template stuff?

(not today, first I'll give some time for more discussions ;) )

@vangnp3702

Copy link
Copy Markdown

@peluko00 I've moved recompute to the agreement_template. agreement_revision and agreement_product depends on template. Moving all the template stuff into agreement module would make things easier, but I did not want to change to much on the agreement base module. The agreement_template feature itself is still controllable by user group, as before.

I still don't see the benefit, why not to move things like revision or stage into separate modules which can be used by agreement, without agreement_legal. If you come to the OCA Days you may explain it to my on a beer.

I think the best way to continue is to split in to two worlds. I'll rename this agreement_legal into agreement_* focusing on my requirements and not on backwards compatibility. I'll mark all the modules in this PR as incompatible with "agreement_legal" for better distinction. I'll also remove agreement_legal_content because this part is ugly anyway.

Someone else can than provide a new 1:1 migration of agreement_legal according to your needs.

@vangnp3702 do you have contact to other agreement module users? Maybe we can discus what else could be added/changed to the agreement module. The settings menu and security roles have already been merged into agreement, but maybe we could also add a kanban view or move some template stuff?

(not today, first I'll give some time for more discussions ;) )

I think you should review the migration script again because translate=True makes agreement_type.name a jsonb column. Currently, your script doesn't handle that right?

@CRogos

CRogos commented Aug 21, 2026

Copy link
Copy Markdown
Author

@vangnp3702 are you on the latest version of v18?
https://github.com/OCA/agreement/blob/18.0/agreement/models/agreement_type.py
There was a recent change to translate=True in v18. #124

I tried to handle both input in the migration, but I failed. Therefore you need to install the latest v18 first.

@CRogos

CRogos commented Sep 3, 2026

Copy link
Copy Markdown
Author

I've created a dedicated PR for each new module. Each module is marked as incompatible with agreement_legal

agreement_template #134
agreement_stage #135
agreement_type #136
agreement_termination #137
agreement_signature #138
agreement_revision #139
agreement_product #140

Because I don't use the functionality of agreement_legal_content and agreement_legal, this is not available as dedicated module.

I will now close this PR so it is not blocking #132

@CRogos CRogos closed this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

migration mod:agreement_legal_content Module agreement_legal_content mod:agreement_legal Module agreement_legal mod:agreement_product Module agreement_product mod:agreement_project Module agreement_project mod:agreement_revision Module agreement_revision mod:agreement_signature Module agreement_signature mod:agreement_stage Module agreement_stage mod:agreement_template Module agreement_template mod:agreement_termination Module agreement_termination mod:agreement_type Module agreement_type mod:agreement Module agreement needs review series:19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.