[MIG] stock_reserve_sale: Migration to 12.0 #620

hzh0292 · 2019-06-16T09:49:29Z

Migration of stock_reserve_sale to v12 but found bugs #619
Ask the great community to help me with the migration.

pedrobaeza · 2019-06-16T10:30:40Z

Please preserve commit history following technical method explained in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0

stock_reserve_sale/__manifest__.py

damendieta · 2019-06-16T10:14:11Z

stock_reserve/model/stock_reserve.py

+##############################################################################
+
+from odoo import models, fields, api, _
+from odoo.exceptions import except_orm


except_orm is deprecated, use UserError or Warning or any that suits your needs from the new api

Is ValidationError the suitable new api?

Yes, but is this a validation error?

I an not sure. What's your advice?

you can remove the import line and leave only the ValueError on line 118
except ValueError:

Because your are not raising the error but catching it.

damendieta · 2019-06-16T10:27:14Z

stock_reserve/model/stock_reserve.py

+        A date until which the product is reserved can be specified.
+        """
+        self.write({'date_expected': fields.Datetime.now()})
+        self.mapped('move_id')._action_confirm()


here you have the problem

I tested the following:

If the product is storable and you have the qty to reserve, everything works.

If the product is consumable it fails.

If the product is storable but you have not enough qty available, it fails again.

So, you need to:

Ad a filter in product to allow only storable products.

Add a constrain in qty to reserve to avoid the user to ask a reservation to more products than available.

Catch the error in order to avoid a problem is anyone else consumes the available qty before you confirm.

When the product is storable and you have the qty to reserve,not really everything works.When I create a sale order which contain two storable products,when I click the reserve stock button to create reversions and add a validity date,it will raise the error.

You are right, I didn't tested that as I thought the problem was the same.

The problem seems to be that you have several records and action confirm fails.

Thy doing a loop, instead of lines 140 and 141 try:

for m in self.mapped('move_id') m._action_confirm() m.picking_id.action_assign()

It's posible that it will fail because you may not have a picking on the move, if that's the case, try:

for m in self.mapped('move_id') m._action_confirm() if m.picking_id: m.picking_id.action_assign()

I was thinking about this and most of the time we will have one picking for all moves or no move at all, so, we can do:

for m in self.mapped('move_id') m._action_confirm() for p in self.mapped('move_id.picking_id') p.action_assign()

OK,I'll try.Thank you very much.

for m in self.mapped('move_id'): m._action_confirm() for p in self.mapped('move_id.picking_id'): p.action_assign()

This didn't solve the issue that create a stock reservation from a sale order with multi lines.

Ok, I checked the problem. The problem is not because of multiple lines.

In v12 _action_confirm is defined like this _action_confirm(self, merge=True, merge_into=False), while in v10, it's def action_confirm(self).

As you can see, you have 2 new params in the definition, one of them is True by default.

For that reason, it merges the move wich deletes the original move.
Try this: m._action_confirm(merge=False, merge_into=False)

Great!!!It works.You are so helpful,thank you very much.

damendieta · 2019-06-16T10:40:54Z

stock_reserve/model/stock_reserve.py

+        """
+        Release moves from reservation
+        """
+        self.mapped('move_id').action_cancel()


Here you have an error with action_cancel, it's _action_cancel, so you need to add the _

damendieta · 2019-06-16T11:23:37Z

@hzh0292 before doing any other commit, please follow @pedrobaeza indications.

Your PR will not be accepted if you don't do it.

hzh0292 · 2019-06-16T11:33:07Z

@hzh0292 before doing any other commit, please follow @pedrobaeza indications.

Your PR will not be accepted if you don't do it.

Yes, I know. Very sorry for that I am not good at this work. I will try.

damendieta · 2019-06-16T17:41:34Z

@hzh0292 I have seen a design flaw on this module, it's designed to "reserve" the quantities, but not for them to be moved to the reservation location. It meas that the stock can be reserved but not moved to a different location.

I't important to have the flow with the option for the move to be confirmed and moved, so, we have two options.

If the move is not confirmed, the actual workflow is fine.
If the move is done, you will be presented with a "You cannot cancel a stock move that has been set to 'Done'. " error, and you will not be able to undo the reservation.

My suggestion will be to add a condition to the code, like this:
First, add float_round and decimal_precision to your imports.

from odoo.addons import decimal_precision as dp
from odoo.tools.float_utils import float_round

Then, replace the release function:

    @api.multi
    def release(self):
        """
        Release moves from reservation
        """
        # First we cancel all reserved moves.
        self.mapped('move_id').filtered(
            lambda x: x.state not in ('done', 'draft', 'cancel')
        )._action_cancel()
        # If the move is done, we need to reverse it.
        done_moves = self.mapped('move_id').filtered(lambda x: x.state == 'done')
        # We group them by picking to have one revesed picking for all moves.
        for picking in done_moves.mapped('picking_id'):
            return_obj = self.env['stock.return.picking']
            product_return_moves = []

            if picking.state != 'done':
                raise UserError(_("You may only return Done pickings."))

            for move in done_moves.filtered(lambda x: x.picking_id == picking):
                if move.scrapped:
                    continue

                move_dest_exists = False
                if move.move_dest_ids:
                    move_dest_exists = True

                quantity = move.product_qty - sum(move.move_dest_ids.filtered(
                    lambda m: m.state in [
                        'partially_available', 'assigned', 'done'
                    ]).mapped('move_line_ids').mapped('product_qty')
                )
                quantity = float_round(
                    quantity,
                    precision_rounding=move.product_uom.rounding
                )
                product_return_moves.append(
                    (0, 0, {
                        'product_id': move.product_id.id,
                        'quantity': quantity,
                        'move_id': move.id,
                        'uom_id': move.product_id.uom_id.id
                    }))

            location_id = picking.location_id.id
            if picking.picking_type_id.return_picking_type_id \
                    .default_location_dest_id.return_location:

                location_id = picking.picking_type_id.return_picking_type_id \
                    .default_location_dest_id.id

            parent_location_id = picking.picking_type_id.warehouse_id \
                and picking.picking_type_id.warehouse_id.view_location_id.id \
                or picking.location_id.location_id.id

            wizard = return_obj.create({
                'picking_id': picking.id,
                'location_id': location_id,
                'product_return_moves': product_return_moves,
                'move_dest_exists': move_dest_exists,
                'parent_location_id': parent_location_id,
                'original_location_id': picking.location_id.id,
            })
            wizard._create_returns()
        return True

This code is working, but I didn't tested it too much.

I hope it helps.

hzh0292 · 2019-06-17T06:33:39Z

@damendieta I have updated the code.Please check it and give me some advice about the code adjustment and the work I should do following odoo 12 migration guidline .

damendieta · 2019-06-17T12:55:53Z

Hi, just follow the steps Pedro give you.

The module is almost complete with the modifications made.

hzh0292 · 2019-06-19T00:57:37Z

Hi, this default has no sense.

If you are asking for a reservation, it's not for the same day you are asking for the reservation.

In case you want a default, you should have a configuration to have a default reservation period on the company or no default at all. Why do you need this?

OK

damendieta · 2019-06-26T19:30:50Z

@hzh0292 please, don't forget to migrate the addons as Pedro told you, so we can merge this PR soon.

I know it's a little work, but it's important to merge the modules.

Thanks.

hzh0292 · 2019-06-27T08:44:33Z

@hzh0292 please, don't forget to migrate the addons as Pedro told you, so we can merge this PR soon.

I know it's a little work, but it's important to merge the modules.

Thanks.

I am so sorry.But I am really don't know which part of work I should do next,so please tell me how can I do to finish it.I have seen the documentation of OCA,but it seems that I have problem of totally following this.

pedrobaeza · 2019-06-27T09:42:27Z

@hzh0292, you must preserve commit history following technical method explained in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0

You will need to start again from a fresh branch, doing that method, and then you can copy current code of this pull request and commit it with only the changes fro 12.0.

hzh0292 · 2019-06-28T06:16:55Z

@hzh0292, you must preserve commit history following technical method explained in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0

You will need to start again from a fresh branch, doing that method, and then you can copy current code of this pull request and commit it with only the changes fro 12.0.

Thank you very much.I have started again.Should I make a new PR?

pedrobaeza · 2019-06-28T07:08:32Z

If you preserve branch name and then forced push here, there's no need of closing the pull request.

hzh0292 · 2019-06-28T07:32:02Z

If you preserve branch name and then forced push here, there's no need of closing the pull request.

But I have deleted the old repo and forked again.

pedrobaeza · 2019-06-28T07:35:29Z

Yeah, in that case you'll need to open a new PR. Closing this one.

[MIG] stock_reserve_sale: Migration to 12.0

b05a2f4

hzh0292 mentioned this pull request Jun 16, 2019

Migration of stock_reserve_sale to v12 but found bugs #619

Closed

pedrobaeza added this to the 12.0 milestone Jun 16, 2019

damendieta suggested changes Jun 16, 2019

View reviewed changes

hzh0292 added 2 commits June 16, 2019 19:12

Update __manifest__.py

db5694a

Update stock_reserve.py

3797485

hzh0292 mentioned this pull request Jun 16, 2019

Migration to version 12.0 #487

Closed

54 tasks

Update stock_reserve.py

1ea4585

hzh0292 and others added 3 commits June 17, 2019 10:39

Update __manifest__.py

a2e8da5

[MIG] stock_reserve_sale: Migration to 12.0

8b4c7fb

[MIG] stock_reserve_sale: Migration to 12.0

2b17e1b

Update stock_reserve.py

7682c4d

rousseldenis added the needs fixing label Jun 18, 2019

hzh0292 added 2 commits June 19, 2019 08:58

Update stock_reserve.py

9d65b50

Update stock_reserve.py

690fd90

pedrobaeza closed this Jun 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MIG] stock_reserve_sale: Migration to 12.0 #620

[MIG] stock_reserve_sale: Migration to 12.0 #620

hzh0292 commented Jun 16, 2019

pedrobaeza commented Jun 16, 2019

damendieta Jun 16, 2019

hzh0292 Jun 16, 2019

damendieta Jun 16, 2019

hzh0292 Jun 16, 2019

damendieta Jun 16, 2019

damendieta Jun 16, 2019 •

edited

hzh0292 Jun 16, 2019

damendieta Jun 16, 2019 •

edited

damendieta Jun 16, 2019 •

edited

hzh0292 Jun 16, 2019

hzh0292 Jun 16, 2019 •

edited

damendieta Jun 16, 2019 •

edited

hzh0292 Jun 16, 2019

damendieta Jun 16, 2019

damendieta commented Jun 16, 2019

hzh0292 commented Jun 16, 2019

damendieta commented Jun 16, 2019 •

edited

hzh0292 commented Jun 17, 2019

damendieta commented Jun 17, 2019

hzh0292 commented Jun 19, 2019

damendieta commented Jun 26, 2019

hzh0292 commented Jun 27, 2019 •

edited

pedrobaeza commented Jun 27, 2019

hzh0292 commented Jun 28, 2019

pedrobaeza commented Jun 28, 2019

hzh0292 commented Jun 28, 2019

pedrobaeza commented Jun 28, 2019

[MIG] stock_reserve_sale: Migration to 12.0 #620

[MIG] stock_reserve_sale: Migration to 12.0 #620

Conversation

hzh0292 commented Jun 16, 2019

pedrobaeza commented Jun 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damendieta Jun 16, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damendieta Jun 16, 2019 • edited

Choose a reason for hiding this comment

damendieta Jun 16, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hzh0292 Jun 16, 2019 • edited

Choose a reason for hiding this comment

damendieta Jun 16, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damendieta commented Jun 16, 2019

hzh0292 commented Jun 16, 2019

damendieta commented Jun 16, 2019 • edited

hzh0292 commented Jun 17, 2019

damendieta commented Jun 17, 2019

hzh0292 commented Jun 19, 2019

damendieta commented Jun 26, 2019

hzh0292 commented Jun 27, 2019 • edited

pedrobaeza commented Jun 27, 2019

hzh0292 commented Jun 28, 2019

pedrobaeza commented Jun 28, 2019

hzh0292 commented Jun 28, 2019

pedrobaeza commented Jun 28, 2019

damendieta Jun 16, 2019 •

edited

damendieta Jun 16, 2019 •

edited

damendieta Jun 16, 2019 •

edited

hzh0292 Jun 16, 2019 •

edited

damendieta Jun 16, 2019 •

edited

damendieta commented Jun 16, 2019 •

edited

hzh0292 commented Jun 27, 2019 •

edited