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

[FIX][14.0] extra price/price computation for configurable products #52

Merged
merged 1 commit into from
Dec 10, 2021
Merged

[FIX][14.0] extra price/price computation for configurable products #52

merged 1 commit into from
Dec 10, 2021

Conversation

pledrateam
Copy link
Contributor

No description provided.

@PCatinean
Copy link
Contributor

@DonatasV @gaikaz can you test this MR on your scenario described in #49 ?

@gaikaz
Copy link
Member

gaikaz commented Oct 28, 2021

Thank you so much for actually getting back with this! Will definitely test it! Hopefully tomorrow.

DonatasV
DonatasV approved these changes Nov 12, 2021
View reviewed changes
Copy link
Contributor

@DonatasV DonatasV 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

@PCatinean
Copy link
Contributor

@pledrateam seems code coverage has failed due to the code removal, can we bump it up slightly so all tests are green?

@DonatasV
Copy link
Contributor

By testing the scenario described in #49, we also found another bug with is related to product_configurator_sale module and the same pricelists... If sale order already has selected pricelist with, for example 50%, discount, then after we use configurator in SO, the price of product will be selected without discount.
That happening because the price is selected from configurator session but ignores pricelist. (https://github.com/OCA/product-configurator/blob/14.0/product_configurator_sale/wizard/product_configurator.py#L26)

@rousseldenis rousseldenis added enhancement New feature or request needs review labels Nov 12, 2021
@pledrateam pledrateam force-pushed the 14.0-extra-price-computation-for-configurable-products branch from f2751d7 to bfe5621 Compare November 23, 2021 05:03
@pledrateam
[WIP][T-01839]product_configurator: remove price calculation from ses…
5508465 
…sion and also get value from get_attribute_value_extra_prices
@pledrateam pledrateam force-pushed the 14.0-extra-price-computation-for-configurable-products branch from b12d1d4 to 5508465 Compare December 2, 2021 09:31
@PCatinean
Copy link
Contributor

@DonatasV we should have fixed also the last issue you mentioned. Can you please confirm?

@DonatasV
Copy link
Contributor

@PCatinean
Sorry to keep you wait. I checked and everything work fine now.

@PCatinean
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Sorry @PCatinean 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.

@PCatinean
Copy link
Contributor

@dreispt is the ocabot maintainer check case sensitive by chance?

@dreispt
Copy link
Member

dreispt commented Dec 10, 2021

@PCatinean it could be the case, I can fix the maintainers key.

@dreispt
Copy link
Member

dreispt commented Dec 10, 2021

After #54 is merged, you can rebase and try again.

@PCatinean
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-52-by-PCatinean-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f6c08e8 into OCA:14.0 Dec 10, 2021
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 4a499c1. 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
Reviewers

@DonatasV DonatasV DonatasV approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request merged 🎉 needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@pledrateam @PCatinean @gaikaz @DonatasV @OCA-git-bot @dreispt @rousseldenis