[16.0][IMP] sale_order_line_cancel: Refactor and create base addon#3894
Conversation
mt-software-de
force-pushed
the
16-refactor-sale_order_line_cancel
branch
4 times, most recently
from
d46d217 to
99603a5
Compare
| ) | ||
| == 1 | ||
| and rec.state in ("sale", "done") | ||
| and rec.qty_delivered_method == "stock_move" |
There was a problem hiding this comment.
This line is now gone, imo it is not needed to check for qty_delivered_method.
| sale_lines = self.env["sale.order.line"] | ||
| for move in self: | ||
| if ( | ||
| move.sale_line_id |
There was a problem hiding this comment.
This check was removed it is already in _is_move_to_take_into_account_for_qty_canceled
| sale_line | ||
| and self.state == "cancel" | ||
| and self.picking_type_id.code == "outgoing" | ||
| and sale_line.state not in ["draft", "sent", "cancel"] |
There was a problem hiding this comment.
This ensures that i can not be called for a canceled order line.
| and self.state == "cancel" | ||
| and self.picking_type_id.code == "outgoing" | ||
| and sale_line.state not in ["draft", "sent", "cancel"] | ||
| and not sale_line._get_moves_to_cancel() |
There was a problem hiding this comment.
This check ensures that there are no other open move lines, like in a mrp scenario.
mt-software-de
force-pushed
the
16-refactor-sale_order_line_cancel
branch
4 times, most recently
from
27ea46c to
1e721c1
Compare
| lines = self.filtered( | ||
| lambda l: l.qty_delivered_method == "stock_move" | ||
| and l.can_cancel_remaining_qty | ||
| ) | ||
| return lines.move_ids.filtered(lambda m: m.state not in ("done", "cancel")) |
There was a problem hiding this comment.
On a performance point of view, this code iterates 2 times on the lines.... We should do it on 1 loop.
There was a problem hiding this comment.
Sorry no. It doesn't, or? :-)
At first it iterates on lines and then on the moves.
There was a problem hiding this comment.
@lmignon Not sure, as at return lines.move_ids... lines are recordset. No more iteration
lmignon
left a comment
lmignon
left a comment
There was a problem hiding this comment.
The changes seem consistent overall. I have a quick question about the reason for no longer using the ‘qty_to_deliver’ attribute...
As for the module names, I find them counterintuitive. I would prefer to keep the basic functionality in sale_order_line_cancel and the code related to stock in sale_stock_sale_order_line_cancel. This seems to me to follow the logic adopted by Odoo.
Because by changing the dependency to only sale in the base addon, it is not available anymore. |
Yes thats what i have written in the description of this PR. The only concern i have about this, whats about other user which are using this addon already? If we are changing the name the initial functionality isn't available anymore only if they know to install the renamed addon. |
mt-software-de
force-pushed
the
16-refactor-sale_order_line_cancel
branch
from
1e721c1 to
1cf272c
Compare
Agree with @lmignon on that. |
OK perfect then i will rename the modules |
mt-software-de
force-pushed
the
16-refactor-sale_order_line_cancel
branch
3 times, most recently
from
5e83699 to
afc2300
Compare
mt-software-de
force-pushed
the
16-refactor-sale_order_line_cancel
branch
from
222ab70 to
b78f15e
Compare
|
This PR has the |
|
/ocabot merge major |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at e6bc28a. Thanks a lot for contributing to OCA. ❤️ |
5 participants
This PR moves the base functionality of
sale_order_line_cancelinto an own addonsale_order_line_cancel_base.This allows it, to use it also without stock modules.
Within this refactoring i changed the logic, when the update of
product_qty_canceled, before it was triggered by canceling a stock move. Now it is directly triggered when someone cancels the remaining qty.This leads to the improvement i wanted, you can cancel the remaining qty of sale.order.lines without a stock.move.
This happens when a sale.order.line creates a not confirmed purchase.order.
sale_order_line_cancelwill be now only adding the functionality to cancel the stock.moves when the cancel on the sale.order.lines is triggered or the other way around.IMO: the naming of the addons should be
sale_order_line_cancelandsale_order_line_cancel_stock, but i don't know if this would be good sincesale_order_line_cancelis already in use.A next task would be to create a addon called
sale_order_line_cancel_purchase.