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] account_payment_term_extension: Add cash_rounding parameter to _compute_terms() to prevent error #758

Conversation

victoralmau
Copy link
Member

@victoralmau victoralmau commented Aug 30, 2024

Add cash_rounding parameter to _compute_terms() to prevent error

Required since odoo/odoo@84b03f7

Please @pedrobaeza can you review it?

@Tecnativa

@victoralmau victoralmau changed the title [16.0][FIX] account_payment_term: Add cash_rounding support in _compute_terms() [16.0][FIX] account_payment_term_extension: Add cash_rounding support in _compute_terms() Aug 30, 2024
@victoralmau victoralmau force-pushed the 16.0-fix-account_payment_term_extension-_compute_terms branch from 7544e4e to 8e580ff Compare August 30, 2024 06:51
@pedrobaeza pedrobaeza added this to the 16.0 milestone Aug 30, 2024
@pedrobaeza
Copy link
Member

This module seems a bit bloated and copying/pasting a lot of code from upstream, while the feature it brings don't require it directly. Can't this be rethought for not requiring this doing some kind of tricks?

@victoralmau
Copy link
Member Author

I agree that currently the _compute_terms() method copies a lot of code and probably can be refactored but, is it the time now? I think not, what is necessary now is to unblock those who use this module and then, create a new PR to refactor this if possible (probably it will be necessary to create extra methods in odoo core).

@pedrobaeza
Copy link
Member

Then just add the parameter, and issue an error if the parameter is used, adding in the ROADMAP that this module is not compatible with cash rounding.

@victoralmau victoralmau force-pushed the 16.0-fix-account_payment_term_extension-_compute_terms branch from 8e580ff to 4fee4cc Compare August 30, 2024 07:46
@victoralmau victoralmau changed the title [16.0][FIX] account_payment_term_extension: Add cash_rounding support in _compute_terms() [16.0][FIX] account_payment_term_extension: Add cash_rounding parameter to _compute_terms() to prevent error Aug 30, 2024
@pedrobaeza
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-758-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit afb6a06 into OCA:16.0 Aug 30, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

4 participants