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.

Sign up for GitHub

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

Jump to bottom

[16.0][MIG] product_variant_sale_price: Migration to 16.0 #290

Merged
merged 33 commits into from
Feb 14, 2024
Merged

[16.0][MIG] product_variant_sale_price: Migration to 16.0 #290

merged 33 commits into from
Feb 14, 2024

Conversation

ecino
Copy link

@ecino ecino commented Apr 13, 2023 •
edited
Loading

Adapted the views and fixed a bug where the sales price on the template was updating with the previous value of a product.

This PR includes also the work done by RabbitJon-S73 in #281 but goes a little bit further by improving the views.

@ecino ecino mentioned this pull request Apr 13, 2023
Open
9 tasks
@ecino ecino force-pushed the 16.0-mig-product_variant_sale_price branch 2 times, most recently from b11c84a to 5a30a94 Compare April 13, 2023 21:07
@ecino ecino mentioned this pull request Apr 14, 2023
Closed
@ecino ecino force-pushed the 16.0-mig-product_variant_sale_price branch from 5a30a94 to 296eff6 Compare April 14, 2023 06:46
@ecino
Copy link
Author

ecino commented Apr 14, 2023

This PR includes also the work done by RabbitJon-S73 in #281 but goes a little bit further by improving the views.

@ecino ecino force-pushed the 16.0-mig-product_variant_sale_price branch 2 times, most recently from 77b700b to 888a1ec Compare April 14, 2023 10:46
@RabbitJon-S73
Copy link
Contributor

@ecino LGTM, ask the other reviewers to have a look and if they find it ok im closing my PR to avoid conflicts.

hitrosol
hitrosol approved these changes Apr 20, 2023
View reviewed changes
Copy link

@hitrosol hitrosol left a comment

Choose a reason for hiding this comment

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

Tested in runboat, LGTM.

@aleuffre
Copy link
Contributor

@ecino Could you rebase and solve the conflicts please?

@aleuffre
Copy link
Contributor

aleuffre commented May 11, 2023 •
edited
Loading

Warning during installation:

2023-05-11 14:43:45,436 20 WARNING devel odoo.api.create: The model odoo.addons.product_variant_sale_price.models.product_product is not overriding the create method in batch

Could you overwrite the method using the @api.model_create_multi decorator? It shouldn't be too much work, and in fact it will make create and write more similar as a result

@ecino ecino force-pushed the 16.0-mig-product_variant_sale_price branch 2 times, most recently from 9e34108 to bfccc61 Compare June 5, 2023 11:15
@ecino
Copy link
Author

ecino commented Jun 5, 2023

Thanks for the reviews and sorry for the delay in my answer. I corrected all reported issues.

aleuffre
aleuffre approved these changes Jun 5, 2023
View reviewed changes
Copy link
Contributor

@aleuffre aleuffre left a comment

Choose a reason for hiding this comment

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

Code review, LGTM

@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). 🤖

@ecino
Copy link
Author

ecino commented Jun 20, 2023

I applied the nice suggestions from @FrankC013 and also added a new Action that can be very handy for controlling which variant will be the price reference in case the user prefers not to have the cheapest variant as the reference price (which is the default behaviour).

@ecino ecino force-pushed the 16.0-mig-product_variant_sale_price branch from c6d7ccf to 39c984c Compare June 20, 2023 06:48
@ecino
Copy link
Author

ecino commented Jun 29, 2023

Is there something else preventing the merge? Can someone give the final review here? Thanks!

@maxgrassi
Copy link

Bug report: Tested and this is not working in version 16: "This module also hides sale price at product template level when has more than one variant."
The product price can be set at product templete level and this overwrites all the prices that have been set at product variant level.

oca-ci and others added 6 commits October 16, 2023 09:22
@ecino
[UPD] Update product_variant_sale_price.pot
4e99c0e
@ajaniszewska-dev @ecino
Translated using Weblate (French)
5075f18 
Currently translated at 83.3% (5 of 6 strings)

Translation: product-variant-15.0/product-variant-15.0-product_variant_sale_price
Translate-URL: https://translation.odoo-community.org/projects/product-variant-15-0/product-variant-15-0-product_variant_sale_price/fr/
@Noel000 @ecino
Translated using Weblate (Catalan)
307c24c 
Currently translated at 100.0% (6 of 6 strings)

Translation: product-variant-15.0/product-variant-15.0-product_variant_sale_price
Translate-URL: https://translation.odoo-community.org/projects/product-variant-15-0/product-variant-15-0-product_variant_sale_price/ca/
@ecino
[IMP] product_variant_sale_price: pre-commit stuff
ac514fd
@ecino
[MIG] product_variant_sale_price: Migration to 16.0
c294576
@ecino
[IMP] product_variant_sale_price: Set price_extra to 0
c801440 
Apply suggested changes from FrankC013
@ecino ecino force-pushed the 16.0-mig-product_variant_sale_price branch from 187a929 to c801440 Compare October 16, 2023 07:22
@ecino
Copy link
Author

ecino commented Oct 16, 2023

PR Fixed.

@LoisRForgeFlow
Copy link

@ecino It seems that tests are failing, can you have a look?

@ecino
Copy link
Author

ecino commented Oct 26, 2023

Apparently it causes problem with the product_variant_name module that was introduced lately. I will check when I have time.

@uvid-sinisa
Copy link

@ecino Looks like module product_variant_name adds field "Name" to product.product as required but tests in product_variant_sale_price create product_products without name.
Would the appropriate fix be to just add some name when creating the product_product in tests?

@ecino
Copy link
Author

ecino commented Dec 11, 2023

Thanks for the hint. Can we merge now?

@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). 🤖

@aleuffre
Copy link
Contributor

aleuffre commented Jan 5, 2024

@OCA/product-maintainers

amokaddem added a commit to archeti-org/product-variant that referenced this pull request Jan 16, 2024
@amokaddem
[28209] [16.0][MIG] product_variant_sale_price: Migration to 16.0 OCA…
08bc2ec 
…#290
amokaddem added a commit to archeti-org/product-variant that referenced this pull request Jan 16, 2024
@amokaddem
Merge pull request #1 from archeti-org/28209_16.0_MIG_product_variant…
a67596f 
…_sale_price

[28209] [16.0][MIG] product_variant_sale_price: Migration to 16.0 OCA#290
@HaraldPanten
Copy link

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Sorry @HaraldPanten you are not allowed to merge.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@HaraldPanten
Copy link

@etobella could you merge this one? I forgot I don't have permissions here! 😂

@etobella
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 33c3aac into OCA:16.0 Feb 14, 2024
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@ecino ecino deleted the 16.0-mig-product_variant_sale_price branch February 15, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FrankC013 FrankC013 FrankC013 approved these changes

@aleuffre aleuffre aleuffre approved these changes

@hildickethan hildickethan hildickethan approved these changes

@HaraldPanten HaraldPanten HaraldPanten approved these changes

@eantones eantones eantones approved these changes

@hitrosol hitrosol hitrosol approved these changes

@manuelcalerosolis manuelcalerosolis manuelcalerosolis approved these changes

@enriquemartin enriquemartin enriquemartin approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.