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.

Sign up for GitHub

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

Jump to bottom

[IMP] Add option to cancel merged source inventories #113

Open
wants to merge 1 commit into
base: 12.0
Choose a base branch
from
Open

[IMP] Add option to cancel merged source inventories #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions stock_inventory_merge/models/wizard_stock_inventory_merge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ class WizardStockInventoryMerge(models.TransientModel):
_description = "Stock Inventory Merge Wizard"

name = fields.Char(string="Inventory Name", required=True)
cancel_src = fields.Boolean(
string="Cancel source inventories?",
default=True,
)

@api.multi
def action_merge(self):
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :

  1. 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 ?
  2. inventories.mapped('move_ids')._action_cancel() seems to be useless (in Odoo core I mean, in action_cancel_draft) because at this step, the inventory is not confirmed, so there are no move_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 !

Copy link
Author

@azoellner azoellner Nov 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

# and set the inventories to 'draft'. We want 'cancel' instead.
inventories.mapped('move_ids')._action_cancel()
inventories.write({'state': 'cancel'})

# Return the action focused on the created inventory
action_data = self.env.ref("stock.action_inventory_form").read()[0]
action_data["res_id"] = inventory.id
Expand Down
7 changes: 5 additions & 2 deletions stock_inventory_merge/views/view_wizard_stock_inventory_merge.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
<field name="model">wizard.stock.inventory.merge</field>
<field name="arch" type="xml">
<form>
<group col="4">
<field name="name"/>
<group>
<group>
<field name="name"/>
<field name="cancel_src" />
</group>
</group>
<footer>
<button name="action_merge" string="Merge Inventories" type="object" class="oe_highlight"/>
Expand Down