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

[MIG] product_pricelist_supplierinfo: Migration to 18.0 #1837

Merged

Conversation

CLaurelB
Copy link

@CLaurelB CLaurelB commented Jan 7, 2025

Supersed #1745

cubells and others added 30 commits January 7, 2025 22:46
* Don't depend on sales
* Extended README
* Tests focused on module specific features
* Code optimization
* Don't mix pricelist info with supplier info on criteria
Date order is passed by context, so we have to take this into account for
computing properly available supplierinfo records.
If not, you can twist the order in the form and rules won't be applied according criteria
If you set the value "Based on" for using supplier info, but then you change the
computation type to another one (like fixed price or discount), this code is still
acting, so we should check both fields.
Currently translated at 93.3% (14 of 15 strings)

Translation: product-attribute-12.0/product-attribute-12.0-product_pricelist_supplierinfo
Translate-URL: https://translation.odoo-community.org/projects/product-attribute-12-0/product-attribute-12-0-product_pricelist_supplierinfo/es/
When having price per qty at the supplier level and if you want to
base your pricelist on the supplier price, now you can
define a margin on the product.supplierinfo
Use standard way to select the seller.

As a result a bug in previous solution is fixed also: before this,
all supplierinfo records having product_tmpl_id set, were considered
as possible price for all product variants, even if product_id was
set also. That resulted in same price for all variants even if
supplierinfo records had different price for all variants.
If we have suppliers in two or more currencies, and we have a pricelist
with an item that is selected the "Prices based on supplier info" option
but the pricelist is in a currency different from the supplierinfo the
compute of the price get the price on the same amount but with a different
currency.
With these changes, we are converting the price to the currency on the
pricelist that is being used.
Also, a test was added to prove the previous behaviour.
Currently translated at 100.0% (20 of 20 strings)

Translation: product-attribute-12.0/product-attribute-12.0-product_pricelist_supplierinfo
Translate-URL: https://translation.odoo-community.org/projects/product-attribute-12-0/product-attribute-12-0-product_pricelist_supplierinfo/fr/
It is possible that this method is called on an inactive template,
in which case product_variant_id returns empty recordset and
_select_seller will raise expected singleton error.
If we have a product with a supplier with a UOM "A" but the product is
sold with a UOM "B" the price we are getting is the same no matter the
UOM. For example:
Product with the price 1200 per Dozen and we sell a 1 unit we are
getting the price of 1200 for that unit when we should be obtaining 100.

With these changes, we are converting the price to the UOM that is being
used on the sale or the default on the product.
Also, a test was added to prove the previous behaviour.
[UPD] Update product_pricelist_supplierinfo.pot

[UPD] README.rst
… when date is datetime.

date may come from context
When we check the price of any product, the date that we pass on the context has to be a Datetime value or None. With this change we avoid to pass a False.

Here you have the reference to the code that check that https://github.com/OCA/OCB/blob/13.0/addons/product/models/product.py#L599-L600

TT29935

product_pricelist_supplierinfo 13.0.1.0.2
TT31816

[UPD] Update product_pricelist_supplierinfo.pot

[UPD] README.rst

[IMP] update dotfiles [ci skip]
In the e-commerce, if we have a pricelist with a rule based on
supplierinfo for a determined supplier partner, such partner could be
not accesible for public users and the affected products could not be
reached.

TT31476
If we don't have a supplierinfo that below 1 unit we won't get any
seller. We want to ensure that the condition is fully ignored to get
whatever price given the other criterias.

TT33659
…sers

0581b66 is not enough on v13 for
avoiding the access error, so we sudoed the whole price fetch operation.

TT36898
mymage and others added 2 commits January 16, 2025 22:07
Currently translated at 100.0% (15 of 15 strings)

Translation: product-attribute-17.0/product-attribute-17.0-product_pricelist_supplierinfo
Translate-URL: https://translation.odoo-community.org/projects/product-attribute-17-0/product-attribute-17-0-product_pricelist_supplierinfo/it/
@CLaurelB CLaurelB force-pushed the 18.0-mig-product_pricelist_supplierinfo-CLaurelB branch from 0f1a8e9 to f35fd64 Compare January 16, 2025 23:15
@CLaurelB CLaurelB requested a review from luisg123v January 16, 2025 23:49
@CLaurelB CLaurelB force-pushed the 18.0-mig-product_pricelist_supplierinfo-CLaurelB branch from f35fd64 to 733ab9b Compare January 21, 2025 08:42
@CLaurelB CLaurelB marked this pull request as draft January 21, 2025 17:42
Changelog:
- Use `is` and `is not` for type comparisons, or `isinstance()` for
  isinstance checks.
- Add a test for the scenario where the pricelist is not based on the
  supplier info, ensuring the price is calculated as expected natively.
- Add a test for the scenario where there are no sellers linked to the
  product.
- Replace the currency rate update method by the native
  `setup_other_currency` method, which requires inheriting from
  `BaseCommon`.

Co-authored-by: mle <[email protected]>
@CLaurelB CLaurelB force-pushed the 18.0-mig-product_pricelist_supplierinfo-CLaurelB branch from 733ab9b to 4174ebc Compare January 21, 2025 17:57
@CLaurelB CLaurelB marked this pull request as ready for review January 21, 2025 18:47
Copy link

@luisg123v luisg123v left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@moylop260 could you review/merge, please?

@luisg123v
Copy link

/ocabot migration product_pricelist_supplierinfo

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Jan 21, 2025
@OCA-git-bot
Copy link
Contributor

The migration issue (#1741) has not been updated to reference the current pull request because a previous pull request (#1745) is not closed.
Perhaps you should check that there is no duplicate work.
CC @marielejeune

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

@moylop260
Copy link
Contributor

Did you check #1745 ?

@CLaurelB
Copy link
Author

Did you check #1745 ?

Hello @moylop260,

Yes, that's why @marielejeune is a co-author of the migration, and the PR description mentions it. I superseded the original PR because there was no recent activity.

Regards.
cc. @luisg123v

@moylop260
Copy link
Contributor

@JordiBForgeFlow @florian-dacosta

Please, review

It is already approved but you had comments in the original PR

@desdelinux
Copy link

@JordiBForgeFlow @florian-dacosta @pedrobaeza this is ready, can you please merge it, please?

@moylop260
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-1837-by-moylop260-bump-nobump, awaiting test results.

@moylop260
Copy link
Contributor

Merging in order to avoid waiting for a long time again to be merged

But if you have any comment we can discuss in a new issue

@OCA-git-bot OCA-git-bot merged commit 159d767 into OCA:18.0 Jan 30, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 33feecb. 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.