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][MIG] product_variant_sale_price #281

Closed

Conversation

RabbitJon-S73
Copy link
Contributor

Standard migration to 16.0.

Added #280

@RabbitJon-S73 RabbitJon-S73 force-pushed the 16.0-mig-product_variant_sale_price branch from f5b7af8 to 35b61ad Compare February 3, 2023 12:39
@RabbitJon-S73 RabbitJon-S73 force-pushed the 16.0-mig-product_variant_sale_price branch from 35b61ad to 945e24d Compare February 13, 2023 07:49
@RabbitJon-S73 RabbitJon-S73 changed the title 16.0 mig product variant sale price [16.0][MIG] product_variant_sale_price Feb 14, 2023
@RabbitJon-S73 RabbitJon-S73 force-pushed the 16.0-mig-product_variant_sale_price branch 2 times, most recently from 0cb268f to e78431b Compare February 20, 2023 14:24
@RabbitJon-S73
Copy link
Contributor Author

RabbitJon-S73 commented Feb 23, 2023

@Kev-Roche there is any issue with the pre-commit in v16.0? It works in my local

@Kev-Roche
Copy link
Contributor

@RabbitJon-S73 I m not the right person for this kind of question sorry ;)

@RabbitJon-S73 RabbitJon-S73 force-pushed the 16.0-mig-product_variant_sale_price branch from e78431b to 33d2918 Compare March 1, 2023 10:23
@RabbitJon-S73
Copy link
Contributor Author

@sergio-teruel could you review?

Copy link

@BT-anieto BT-anieto left a comment

Choose a reason for hiding this comment

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

Thanks for your work! Some changes to be done, since there are some changes to do in the migration which have not been applied. Check out the migration page Check https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0

On the other hand remember to squash the commits as indicted in https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate

Comment on lines 37 to 40
if self.env.context.get("uom"):
context_uom = uom_model.browse(self.env.context.get("uom"))
price = product.uom_id._compute_price(price, context_uom)

Choose a reason for hiding this comment

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

Why change? We should minimise the code changes in a migration and the code was proper?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#280 I took this changes, I think is the correct behaviour

Choose a reason for hiding this comment

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

Great! Thank you very much. Let me check.

By the way, tests are not passing. I will rereview when they pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, I solved the problem with the test. I also saw your review on #280 and I changed the .get after being once checked

Choose a reason for hiding this comment

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

Thx @RabbitJon-S73. I asked a question in #280 because I was a bit surprised about the change. Still not an answer, but the code looks good to me :)

product_variant_sale_price/models/product_product.py Outdated Show resolved Hide resolved
product_variant_sale_price/tests/test_product_product.py Outdated Show resolved Hide resolved
product_variant_sale_price/tests/test_product_product.py Outdated Show resolved Hide resolved
@RabbitJon-S73 RabbitJon-S73 force-pushed the 16.0-mig-product_variant_sale_price branch 7 times, most recently from 89a8a53 to a43bec5 Compare March 17, 2023 12:34
Copy link

@BT-anieto BT-anieto left a comment

Choose a reason for hiding this comment

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

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

Copy link

@antoniocanovas antoniocanovas left a comment

Choose a reason for hiding this comment

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

Functional review OK

@ecino
Copy link

ecino commented Apr 14, 2023

Oh, I didn't see this one because it's not linked in the migration issue #272 . I did also a migration myself in which I also included some view changes that I think make sense and help better use the functionality. However here there are some changes in the uom computation part that I think are good. Would you like to review and integrate my changes as well in your PR? See #290

@ecino
Copy link

ecino commented Apr 14, 2023

I went ahead and integrated your corrections into my PR (by cherry-picking and rebasing).

@hugo-cordoba
Copy link

LGTM! (code review)

Apply some PEP8 cleanup

Avoid redundant loop
sergio-teruel and others added 13 commits June 7, 2023 14:01
[NEW][8.0] product_variant_sale_price: Improvements

[NEW][8.0] product_variant_sale_price: Fix travis

OCA Transbot updated translations from Transifex
…ate is created

[product_variant_sale_price] Set minimun fix_price among variants to ensure consistency with the price shown in the shop

OCA Transbot updated translations from Transifex
…ariants

Steps to reproduce:
- create more than one variant
- change fix_price of one variant

Current behavior:
- lst_price of all variants are changed

Expected behavior:
- lst_price of only selected variant is changed
[UPD] Update product_variant_sale_price.pot

[UPD] README.rst

Translated using Weblate (Dutch)

Currently translated at 100,0% (4 of 4 strings)

Translation: product-variant-11.0/product-variant-11.0-product_variant_sale_price
Translate-URL: https://translation.odoo-community.org/projects/product-variant-11-0/product-variant-11-0-product_variant_sale_price/nl/

Update translation files

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

[UPD] README.rst
[UPD] Update product_variant_sale_price.pot

Update translation files

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

Translation: product-variant-12.0/product-variant-12.0-product_variant_sale_price
Translate-URL: https://translation.odoo-community.org/projects/product-variant-12-0/product-variant-12-0-product_variant_sale_price/

Update translation files

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

Translation: product-variant-12.0/product-variant-12.0-product_variant_sale_price
Translate-URL: https://translation.odoo-community.org/projects/product-variant-12-0/product-variant-12-0-product_variant_sale_price/

Translated using Weblate (Portuguese)

Currently translated at 100.0% (6 of 6 strings)

Translation: product-variant-12.0/product-variant-12.0-product_variant_sale_price
Translate-URL: https://translation.odoo-community.org/projects/product-variant-12-0/product-variant-12-0-product_variant_sale_price/pt/
[UPD] Update product_variant_sale_price.pot

[UPD] README.rst

Translated using Weblate (Portuguese)

Currently translated at 100.0% (6 of 6 strings)

Translation: product-variant-14.0/product-variant-14.0-product_variant_sale_price
Translate-URL: https://translation.odoo-community.org/projects/product-variant-14-0/product-variant-14-0-product_variant_sale_price/pt/
[UPD] Update product_variant_sale_price.pot

Translated using Weblate (French)

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/

Translated using Weblate (Catalan)

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/
@RabbitJon-S73 RabbitJon-S73 force-pushed the 16.0-mig-product_variant_sale_price branch from a43bec5 to bd500ad Compare June 7, 2023 12:02
Comment on lines 14 to 19
@api.model
def create(self, vals):
product_tmpl = super().create(vals)
product_tmpl._update_fix_price(vals)
return product_tmpl
Copy link
Member

Choose a reason for hiding this comment

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

better as multi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RabbitJon-S73 RabbitJon-S73 force-pushed the 16.0-mig-product_variant_sale_price branch from bd500ad to 4e0a63c Compare June 22, 2023 11:32
@ecino
Copy link

ecino commented Jun 22, 2023

@RabbitJon-S73 I thought you would close this one in favor of #290

@maxgrassi
Copy link

maxgrassi commented Jul 9, 2023

Version of studio73
Bug report: Does not manage prices at variant level.

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.