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][FIX] sale_margin_delivered: Avoid rounding error #214

Conversation

lmignon
Copy link

@lmignon lmignon commented Jun 18, 2024

'price_reduce' is deprecated and removed into the next version. Compute
the prirce_reduct from the price_subotal / product_uom_qty. We might
be tempted to use the 'price_reduce_taxecl' field from the sale order
line but this field is rounded by default to the monetary precision.

As an additional benefit this change ensures the compatibility with
the 'sale_triple_discount' addon. Indeed, when
'sale_triple_discount' is installed, the discount field is not used
as an aggregation of all the applied discount. It's only use to store
the first discount applied. Therefore, the field is not properly
computed since it doesn't include the second and third discount.

@OCA-git-bot
Copy link
Contributor

Hi @sergio-teruel,
some modules you are maintaining are being modified, check this out!

@lmignon
Copy link
Author

lmignon commented Jun 19, 2024

@Shide @yajo Can you take a look at this one plz?

Copy link

@Shide Shide left a comment

Choose a reason for hiding this comment

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

Wonderful, checked code and does the same

@lmignon lmignon marked this pull request as draft June 19, 2024 13:11
@lmignon
Copy link
Author

lmignon commented Jun 19, 2024

@Shide I'll change this since price_reduce_taxexcl is rounded and therefore the margin is not properly computed when we've more than 1 product.

@lmignon lmignon force-pushed the 16.0-sale-margin-deliverd-uses-right-reduce-price-lmi branch from e289eb6 to 4514ace Compare June 19, 2024 13:55
'price_reduce' is deprecated and removed into the next version. Compute
the prirce_reduct from the price_subotal / product_uom_qty. We might
be tempted to use the 'price_reduce_taxecl' field from the sale order
line but this field is rounded by default to the monetary precision.

As an additional benefit this change ensures the compatibility with
the 'sale_triple_discount' addon. Indeed, when
'sale_triple_discount' is installed, the discount field is not used
as an aggregation of all the applied discount. It's only use to store
the first discount applied. Therefore, the field is not properly
computed since it doesn't include the second and third discount.
@lmignon lmignon force-pushed the 16.0-sale-margin-deliverd-uses-right-reduce-price-lmi branch from 4514ace to 461a076 Compare June 19, 2024 13:59
@lmignon
Copy link
Author

lmignon commented Jun 19, 2024

@Shide @yajo @Gelojr @fcvalgar In Odoo standard margin in % is expressed as a decimal... 1 = 100%, 0.5=50% ... In this module margin % is expressed as 100 = 1000%, 50 = 50%... what's the motivation. It's strange to store precentage value in a different way than Odoo. Can I change this and provide a migration script?

@lmignon lmignon marked this pull request as ready for review June 19, 2024 14:22
Put all the new fields after the orginal margin fields from odoo. (prior to this change, the new fields were displayed among the margin fields from Odoo
@lmignon lmignon force-pushed the 16.0-sale-margin-deliverd-uses-right-reduce-price-lmi branch from 461a076 to 6f36c4f Compare June 19, 2024 14:24
@lmignon lmignon changed the title [16.0][FIX] sale_margin_delivered: Uses the right field to get price reduce [16.0][FIX] sale_margin_delivered: Avoid rounding error Jun 19, 2024
Copy link

@fcvalgar fcvalgar left a comment

Choose a reason for hiding this comment

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

I can't see the percentage symbol in the delivered margin.

image

@Shide
Copy link

Shide commented Jun 19, 2024

@Shide @yajo @Gelojr @fcvalgar In Odoo standard margin in % is expressed as a decimal... 1 = 100%, 0.5=50% ... In this module margin % is expressed as 100 = 1000%, 50 = 50%... what's the motivation. It's strange to store precentage value in a different way than Odoo. Can I change this and provide a migration script?

Sure!

@rafaelbn
Copy link
Member

@Shide @yajo @Gelojr @fcvalgar In Odoo standard margin in % is expressed as a decimal... 1 = 100%, 0.5=50% ... In this module margin % is expressed as 100 = 1000%, 50 = 50%... what's the motivation. It's strange to store precentage value in a different way than Odoo. Can I change this and provide a migration script?

This would be an amazing great contribution @lmignon ! 💪🏼 😄 Thank you!

@rafaelbn rafaelbn added this to the 16.0 milestone Jun 19, 2024
Copy link

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

@Shide
Copy link

Shide commented Jun 24, 2024

Could be closed in favor of #215

@lmignon
Copy link
Author

lmignon commented Jun 24, 2024

included into #215

@lmignon lmignon closed this Jun 24, 2024
@lmignon lmignon deleted the 16.0-sale-margin-deliverd-uses-right-reduce-price-lmi branch June 24, 2024 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants