-
-
Notifications
You must be signed in to change notification settings - Fork 542
[16.0][IMP] l10n_es_sigaus_sale: call sigaus from sale line creation #4108
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] l10n_es_sigaus_sale: call sigaus from sale line creation #4108
Conversation
bb1274d
to
6146074
Compare
@Tisho99 revisa los tests antes de los pings please. |
0417bf8
to
03f4529
Compare
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.
@Tisho99 LGTM! Tested on local enviroment.
78929e7
to
4e6a9d4
Compare
4e6a9d4
to
85c5455
Compare
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.
Technical review. LGTM
lines = super().create(vals_list) | ||
if not self.env.context.get("avoid_line_recursion"): | ||
sales = self.env["sale.order"] | ||
for line in lines.filtered(lambda li: li.product_id.sigaus_has_amount): |
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.
sales = lines.filtered(lambda li: li.product_id.sigaus_has_amount).mapped('order_id')
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.
some comments
sales += line.order_id | ||
for sale in sales: | ||
sale.automatic_sigaus_exception() | ||
sale.apply_sigaus() |
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.
apply_sigaus is multi.
for line in lines.filtered(lambda li: li.product_id.sigaus_has_amount): | ||
sales += line.order_id | ||
for sale in sales: | ||
sale.automatic_sigaus_exception() |
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.
This function could be implemented in multiple ways, not just api.one. Can you review it?
The call can be made much better if the change is made.
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.
I can, i'll do the improvement tomorrow
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.
you will have to check which other sites are affected.
85c5455
to
fc302d1
Compare
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.
LGTM
This PR has the |
@HaraldPanten could you do a review? |
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.
/ocabot merge patch
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 207d857. Thanks a lot for contributing to OCA. ❤️ |
There are situations when lines are created directly from the sale order line create method, and the sigaux is not updated because the create or write methods of the sale order are not called.
This PR covers those cases
Related with OCA/sale-workflow#3658
I-7204