[IMP] Add option to cancel merged source inventories #113
Conversation
Codecov Report
@@ Coverage Diff @@
## 12.0 #113 +/- ##
==========================================
+ Coverage 72.31% 72.39% +0.07%
==========================================
Files 66 66
Lines 1510 1514 +4
==========================================
+ Hits 1092 1096 +4
Misses 418 418
Continue to review full report at Codecov.
|
|
Hi @azoellner ! Thanks a lot for your 2 contributions ! kind regards. |
There was a problem hiding this comment.
A design remark. Otherwise, LGTM, thanks !
-
you can add you in the author / credit of the module if you want.
-
As this module seems interesting for other people (out of GRAP), we can imagine share this module in the OCA repo : github.com/oca/stock-logistics-warehouse What do you think ?
| @@ -51,6 +55,12 @@ def action_merge(self): | |||
| for line in inventories.mapped("line_ids"): | |||
| line.copy(default={"inventory_id": inventory.id}) | |||
|
|
|||
| if self.cancel_src: | |||
| # Note: `inventories.action_cancel_draft()` would remove the lines | |||
There was a problem hiding this comment.
Hi. regarding that point, what do you think about a simple OCA module stock_inventory_draft_no_unlink that overwrite the function action_cancel_draft() like
def action_cancel_draft(self):
self.mapped('move_ids')._action_cancel()
self.write({
'state': 'draft'
})
And then, here, only call action_cancel_draft(). I see that change quite generic and not linked to that specific module (stock_inventory_merge)
There was a problem hiding this comment.
Yes, that's right. But be aware that in my PR, we would like to have state cancel, not draft. But this could be easily solved.
I don't know the rationale of the Odoo core team for removing the lines when resetting the inventory to draft.
I mean, if the user wants to reset the inventory, they could always remove the lines manually afterwards.
IMHO this method violates the single-responsibility principle.
So yes, such a module "stock_inventory_draft_no_unlink" might be in order. Possibly providing also a separate method action_cancel() for model stock.inventory.
def action_cancel(self):
self.mapped('move_ids')._action_cancel()
self.write({
'state': 'cancel'
})
def action_cancel_draft(self):
self.action_cancel()
self.write({
'state': 'draft'
})
Also, I agree that this module "stock_inventory_merge" itself could also make a nice addition to OCA's stock-logistics-warehouse.
There was a problem hiding this comment.
Hi, thanks for your answer, and sorry again for the delay.
IMHO this method violates the single-responsibility principle.
Indeed !
well, I think that there is a little problem on Odoo / Core :
- State 'draft' seems to be a "dead" state. I mean, it exist in the selection, but I don't see any place where an inventory can reach to that state (in Odoo Core). maybe it has been kept for legacy reason. What do you think ?
inventories.mapped('move_ids')._action_cancel()seems to be useless (in Odoo core I mean, inaction_cancel_draft) because at this step, the inventory is not confirmed, so there are nomove_ids. What do you think ?
Due to 1. if we set the inventory to the "cancel" state, it is not possible to reset to draft. (except if we implement an other function "action_reset_to_draft") but we can also assume that the inventory is definitively canceled.
so i'm quite perplex !
Except of that, 👍 for that PR. Thanks !
There was a problem hiding this comment.
Thinking about it again, the state 'draft' normally occurs only when initially creating a new "stock.inventory". Such an inventory does not have any lines yet. Only when starting it, which means progressing into state 'confirm' = "In Progess", the user can add lines.
So it makes indeed sense to remove the lines when resetting into state draft.
However, when merging inventories, the original ones should (IMHO) keep their lines. Consequently, this would then mean to not reset them back to state 'draft', but rather to keep them in 'confirm'. Or set them to 'cancel', as per the suggested option.
Would you agree?
This would then mean
def action_cancel(self):
# cancel all moves
self.mapped('move_ids')._action_cancel()
self.write({
'state': 'cancel',
})
def action_cancel_draft(self):
# first cancel
self.action_cancel()
# then set into 'draft', which implies inventory has no lines
self.write({
'line_ids': [(5,)],
'state': 'draft'
})Regarding your second point: Yes, for confirmed, but not yet validated inventories, the self.mapped('move_ids')._action_cancel() is useless, since the inventory does not yet have any stock moves.
However, the method may also be called on validated inventories, and in this case cancelling the stock moves is necessary.
For module "stock_inventory_merge" in version 12.0, this branch adds the option to cancel the merged source inventories.
The "Merge Inventories" wizard will have a checkbox "Cancel source inventories?" When checked, the source inventories will get into state
"cancel"during the merge.Note that the Odoo core method
action_cancel_draft()sets the stock.inventory to state"draft"and deletes the inventory lines:https://github.com/odoo/odoo/blob/6b24df0e037d64be0d4e48045e47215059bffddb/addons/stock/models/stock_inventory.py#L209-L214
But instead, we want to have state
"draft"and keep the lines.