-
-
Notifications
You must be signed in to change notification settings - Fork 181
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] IMP account_product_fiscal_classification : recover classification (or create on the fly) if not set #441
Conversation
@bealdav, @mourad-ehm : could you take a look on this one ? |
Squash by author ? |
Thanks for your review ! Did you tested or just reviewed ? thanks !
Not sure it makes sense. we made a lots of contradictory changes, so I'd be more in favor of squashing in 1 commit. |
I've just made code review. Ok for squash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code review
This PR has the |
fe0111a
to
0e14bdd
Compare
- In a context of product creation, if classification is not provided (in import context for exemple), link the products to an existing classification, if the taxes match an existing one. If no classification match, try to create a new classification, based on the product fiscal settings. (It will fail if user is not member of Accountant group that can create / write on fiscal classifications) - In a context of product update, apply same logic, linking to existing classification, or creating new classification This will fix current limitation if account_product_fiscal_classification and pos_sale (for exemple) is installed, there is an error when making an update, because loading data / demo is failing for the time being if the fields taxes_id or supplier_taxes_id is defined. See for exemple : pos_sale.default_downpayment_product that contains {'taxes_id': '[(5,)]'} write.
0e14bdd
to
da630aa
Compare
Thanks for your review ! /ocabot merge minor |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 6988cf7. Thanks a lot for contributing to OCA. ❤️ |
Supersed #435
@bealdav, could you review this one ?
Note : once approved, we should squash all the commits before merging.
thanks !