Skip to content

[17.0][MIG] sale_product_set: Migration to 17.0#2886

Merged
OCA-git-bot merged 67 commits intoOCA:17.0from
NICO-SOLUTIONS:17.0-mig-sale_product_set
Mar 26, 2024
Merged

[17.0][MIG] sale_product_set: Migration to 17.0#2886
OCA-git-bot merged 67 commits intoOCA:17.0from
NICO-SOLUTIONS:17.0-mig-sale_product_set

Conversation

@NICO-SOLUTIONS
Copy link
Member

standard migration to 17.0

@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 17.0-mig-sale_product_set branch 4 times, most recently from 45d995c to 8c84448 Compare January 9, 2024 15:01
@NICO-SOLUTIONS
Copy link
Member Author

NICO-SOLUTIONS commented Jan 9, 2024

@NICO-SOLUTIONS NICO-SOLUTIONS marked this pull request as ready for review January 9, 2024 16:44
@rousseldenis
Copy link
Contributor

/ocabot migration sale_product_set

@MohamedOsman7
Copy link
Contributor

Code LGTM. The failed check needs to be resolved in order to properly test the functionality using the runboat.

@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 17.0-mig-sale_product_set branch from 123995a to 42cf473 Compare March 13, 2024 17:40
@NICO-SOLUTIONS
Copy link
Member Author

NICO-SOLUTIONS commented Mar 13, 2024

Code LGTM. The failed check needs to be resolved in order to properly test the functionality using the runboat.

The test fails, because product_set is not merged yet and test-requirements is using this PR for it. I rebased it for rebuilding runboat. Feel free to check it

Copy link
Contributor

@MohamedOsman7 MohamedOsman7 left a comment

Choose a reason for hiding this comment

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

code & functional review LGTM

@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 17.0-mig-sale_product_set branch from 42cf473 to 4950f5f Compare March 15, 2024 10:24
Copy link
Contributor

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

Could you also cleanup the commit history?

And I think I've found a bug.
When you switch between product sets, The product list below is not updating.
image

manuelregidor and others added 4 commits March 26, 2024 10:34
Currently translated at 36.1% (13 of 36 strings)

Translation: sale-workflow-15.0/sale-workflow-15.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-15-0/sale-workflow-15-0-sale_product_set/it/
Currently translated at 58.3% (21 of 36 strings)

Translation: sale-workflow-15.0/sale-workflow-15.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-15-0/sale-workflow-15-0-sale_product_set/sl/
Copy link
Contributor

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

The above mentioned behavior is already in Odoo 16.

Christopher Rogos and others added 4 commits March 26, 2024 11:08
[UPD] Update sale_product_set.pot

[UPD] README.rst

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_set/

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_set/

Translated using Weblate (Spanish)

Currently translated at 100.0% (23 of 23 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_set/es/

[UPD] README.rst
Translated using Weblate (Italian)

Currently translated at 100.0% (23 of 23 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_set/it/

[UPD] Update sale_product_set.pot

[BOT] post-merge updates

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_set/

Translated using Weblate (Italian)

Currently translated at 100.0% (26 of 26 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_set/it/

Translated using Weblate (Spanish)

Currently translated at 100.0% (26 of 26 strings)

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_set/es/
… product_set

The reason to move this logic is that there are other modules that extend
product_set as for example stock_product_set but at the same time make use
of the transient model to define a wizard. For this reason it is better to
have the logic available in product_set module and avoid duplicating code or
inheriting from sale_product_set and what this implies in their respective
dependencies with the only need to extend the transient model.
In addition, the transient model is renamed to make it clearer to identify
that it is this type of model.

TT48100

[UPD] Update sale_product_set.pot

[BOT] post-merge updates

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-workflow-16.0/sale-workflow-16.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-16-0/sale-workflow-16-0-sale_product_set/
@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 17.0-mig-sale_product_set branch from 4950f5f to f1c8549 Compare March 26, 2024 10:09
@NICO-SOLUTIONS NICO-SOLUTIONS requested a review from CRogos March 26, 2024 10:12
@NICO-SOLUTIONS
Copy link
Member Author

NICO-SOLUTIONS commented Mar 26, 2024

The above mentioned behavior is already in Odoo 16.

you mean that the lines are not updating is still present in V16? i´ll have a look later on. Here it does now

Copy link
Contributor

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

LGTM code + tested.
(I did the Odoo 16 test with our DB, and at least we have this issue. But maybe we are not on the latest version.)

Just a question... shouldn't we move this button?
image
Not necessary as part of this migration but in general I think it would be much more natural there?

@NICO-SOLUTIONS
Copy link
Member Author

Yeah I like the idea. But I guess there will be be returning questions why this disappeared 😉
But you're right. It would fit better

@CRogos
Copy link
Contributor

CRogos commented Mar 26, 2024

Yeah I like the idea. But I guess there will be be returning questions why this disappeared 😉 But you're right. It would fit better

maybe, but when changing a major version there are also a lot other changes and the user is used to it. 😉

@NICO-SOLUTIONS
Copy link
Member Author

True 😉
I'll attack this soon.

@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 17.0-mig-sale_product_set branch 2 times, most recently from e9fa920 to ec851a8 Compare March 26, 2024 18:57
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.

You can't put the button as another option of adding lines in the one2many, as the way this is built doesn't allow to do that I'm afraid.

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-2886-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit e2e3821 into OCA:17.0 Mar 26, 2024
@OCA-git-bot
Copy link
Contributor

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

@NICO-SOLUTIONS NICO-SOLUTIONS deleted the 17.0-mig-sale_product_set branch March 26, 2024 20:04
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.