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

[16.0][UPD] sale_product_set: Extracted product_set #2604

Merged
merged 64 commits into from
Jul 25, 2023

Conversation

CRogos
Copy link
Contributor

@CRogos CRogos commented Jul 13, 2023

Splitting sale_product_set into product_set and sale_product_set as suggested by @pedrobaeza in #2582

I've left the translations in both modules. Is there a easy way to clean these up what belongs to product_set and what to sale_product_set?

Addresses #1859
Depends on OCA/product-attribute#1384

Pierre Verkest and others added 30 commits June 23, 2023 09:58
* when a set is added to a sales order, it passes the unit of measure
of the product to the sales order line. Fixes an incompatibility with
module sale_margin.
…#543)

* [imp] move SO `add set` button to smartbutton header

* [fix] sale_product_set: PEP8
Currently translated at 100.0% (22 of 22 strings)

Translation: sale-workflow-11.0/sale-workflow-11.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-11-0/sale-workflow-11-0-sale_product_set/es/
Co-Authored-By: sbejaoui <souheil_bejaoui@hotmail.fr>
Currently translated at 100.0% (24 of 24 strings)

Translation: sale-workflow-12.0/sale-workflow-12.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_product_set/es/
Currently translated at 100.0% (25 of 25 strings)

Translation: sale-workflow-12.0/sale-workflow-12.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_product_set/de/
Currently translated at 100.0% (27 of 27 strings)

Translation: sale-workflow-12.0/sale-workflow-12.0-sale_product_set
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_product_set/zh_CN/
Add `partner_id` to product.set allowing to define specific sets per customer
Before this change: try to delete a set used in a wizard right after
-> KABOOM! The FK over the set would prevent deletion
@simahawk
Copy link
Contributor

@CRogos build is 🔴
I guess you forgot to add the pending dependency in test-requirements.txt

Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

Migration scripts are missing

@CRogos CRogos force-pushed the 16.0-imp-sale_product_set-split branch from 62126ef to 33d4ec2 Compare July 18, 2023 08:38
@CRogos
Copy link
Contributor Author

CRogos commented Jul 18, 2023

Migration scripts are missing

I need more info what should be in the migration script.
product_set should be installed by the manifest dependencies.
Is there a problem with the data when migrating?

@CRogos CRogos force-pushed the 16.0-imp-sale_product_set-split branch 4 times, most recently from 9950608 to 2952639 Compare July 18, 2023 09:27
@pedrobaeza
Copy link
Member

Migration scripts are missing

Check my comment at #2582 (comment)

@CRogos CRogos force-pushed the 16.0-imp-sale_product_set-split branch from 2952639 to 660ae50 Compare July 18, 2023 11:05
@CRogos
Copy link
Contributor Author

CRogos commented Jul 18, 2023

@lmignon can you check your translation concerns with this database? I have had a look on the German translation and everything looked good to me.
As Petro said, the PO files were fixed during the merge. Therefore a test with the code of my branch might lead to an false test result.

Currently I see no need for a migration script. Maybe as mentioned before some custom translations might not be migrated correctly for textkeys which moved from sale_product_set to product_set. But as the most UI element stay in sale_product_set and only very few people will actually change these text keys, I think it is fine without script.

@CRogos
Copy link
Contributor Author

CRogos commented Jul 21, 2023

@Yadier-Tecnativa could you review this as well?

@CRogos
Copy link
Contributor Author

CRogos commented Jul 25, 2023

@lmignon and @pedrobaeza can we merge and finish this PR?

@pedrobaeza pedrobaeza added this to the 16.0 milestone Jul 25, 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.

Thanks!

@simahawk
Copy link
Contributor

Cool! Thank you all for your collaboration :)
#ILoveOpenSource ❤️

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2604-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 77b36c5 into OCA:16.0 Jul 25, 2023
@OCA-git-bot
Copy link
Contributor

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

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.