Migration to version 11.0

[max3903]

Moved to https://github.com/OCA/ddmrp/issues/7

[bodedra]

base_cron_exclusion PR#1156

[bodedra]

mrp_mto_with_stock PR#255

[LoisRForgeFlow]

there is no need to migrate web_tree_many2one_clickable, see https://github.com/OCA/web/issues/872. In case of ddmrp this is used just because of one field it is not intended to apply to all M2O fields.

[guewen]

@lreficent hi, could you create the 11.0 branch please? Should I take the 10.0 as base for starting the migration to 11.0 and then apply the changes from #25 or start directly from #25 ?

[LoisRForgeFlow]

@guewen I'm finishing the migration to v10, I'll create the v11 branch when ready. I expect it to be done today.

[guewen]

@lreficent great! thanks, looking forward to start on v11 :)

[guewen]

@lreficent maybe you already saw it, but in case the write method is defined 2 times here https://github.com/Eficent/ddmrp/blob/2a5e0c39472a010c0b68b9f27318c92be5419d22/ddmrp/models/procurement_order.py#L40 :)

[guewen]

@lreficent @jbeficent do you have some recommendation for the migration of the ddmrp module to v11, especially regarding the removal of procurement.order? Maybe you already had some ideas :)

[LoisRForgeFlow]

@guewen branch 11 available https://github.com/Eficent/ddmrp/tree/11.0

[JordiBForgeFlow]

@guewen @lreficent I think that we should be targeting directly to OCA for v11.

@guewen Certainly what will influence the most will be procurements. But should not be a big deal. I think that in general it will be quite straightforward.

[guewen]

@jbeficent No problem for the PR on OCA, but the branch does not exist yet.

I'm happy to read that it should be straightforward for you, to me it's rather intimidating :) Can I ask you some guidance please? I propose that I start by pointing to the different places I spotted, with my analysis if any, on which you can comment if you know what to do.

A. orderpoint_id in purchase.order.line

The orderpoint is found from the procurement.order in a computed field:

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/purchase_order.py#L64-L68

Action: In 11.0, the orderpoint_id is now a core field in purchase.order.line, so I guess this computed field is not needed and we can remove all the related methods.

B. Override of ProcurementOrder._procure_orderpoint_confirm

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/procurement_order.py#L30-L37

Action: The original method has been moved to procurement.group, move it there.

C. ProcurementOrder.add_to_net_flow_equation

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/procurement_order.py#L13-L17

It's related to StockWarehouseOrderpoint.subtract_procurements

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/stock_warehouse_orderpoint.py#L373-L388

And StockWarehouseOrderpoint._calc_net_flow_position

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/stock_warehouse_orderpoint.py#L531-L546

Action: If there is no longer procurements, should we just remove this field from the equation? Replace by stock moves based on states/domain? (seems to me that we only have to remove the field and code related to that)

D. Override of ProcurementOrder.write

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/procurement_order.py#L39-L51

Action: Not sure yet. I removed it but...

E. procurement_ids and orderpoint_id in mrp.production

The orderpoint is found from the procurement orders. We have to find from where we can know it now.

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/mrp_production.py#L91-L98

Action: from my research: there is no orderpoint_id field in mrp.production core, but! the orderpoint is already passed down to the procurement group (https://github.com/odoo/odoo/blob/a2d554c0a55eb00002a22434606ffe0e48d3782b/addons/stock/models/stock_warehouse.py#L831) down to the method StockWarehouseOrderpoint._run_manufacture (https://github.com/odoo/odoo/blob/a2d554c0a55eb00002a22434606ffe0e48d3782b/addons/mrp/models/procurement.py#L24-L26). So it seems that adding the field orderpoint_id to mrp.production and adding the field in ProcurementRule._prepare_mo_vals will automatically get it filled with the orderpoint_id (I have still to check this one).

F. StockWarehouseOrderpoint procurement recommended quantity

The procurement recommended quantity is computed from the procurements linked to a warehouse.

It of course depends on the procurement.order and on a method subtract_procurements_from_orderpoints which doesn't exist anymore. Should it be based on stock moves now?

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/stock_warehouse_orderpoint.py#L113-L159 https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/models/stock_warehouse_orderpoint.py#L302-L304

Action: Only remove the code related to procurement order and keep the rest? Something like:

    @api.multi
    @api.depends("net_flow_position", "dlt", "adu",
                 "buffer_profile_id.lead_time_id.factor",
                 "red_zone_qty", "order_cycle", "minimum_order_quantity",
                 "qty_multiple", "product_uom", "procure_uom_id",
                 "product_uom.rounding")
    def _compute_procure_recommended_qty(self):
        for rec in self:
            procure_recommended_qty = 0.0
            if rec.net_flow_position < rec.top_of_yellow:
                qty = rec.top_of_green - rec.net_flow_position
                if qty >= 0.0:
                    procure_recommended_qty = qty
            if procure_recommended_qty > 0.0:
                reste = rec.qty_multiple > 0 and \
                    procure_recommended_qty % rec.qty_multiple or 0.0

                if rec.procure_uom_id:
                    rounding = rec.procure_uom_id.rounding
                else:
                    rounding = rec.product_uom.rounding

                if float_compare(
                        reste, 0.0,
                        precision_rounding=rounding) > 0:
                    procure_recommended_qty += rec.qty_multiple - reste

                if rec.procure_uom_id:
                    product_qty = rec.product_id.uom_id._compute_quantity(
                        procure_recommended_qty, rec.procure_uom_id)
                else:
                    product_qty = procure_recommended_qty
            else:
                product_qty = 0.0

            rec.procure_recommended_qty = product_qty

G. Blocking of wizard to compute procurement:

https://github.com/Eficent/ddmrp/blob/54c2251b55519fbbffff4fe2a0cdf0634680cb32/ddmrp/wizards/schedulers_all.py#L10-L16

Action: move to 'stock.scheduler.compute'

[guewen]

Here is my progress so far: https://github.com/Eficent/ddmrp/compare/11.0...guewen:mig-ddmrp?expand=1

[guewen]

List of additional pull requests required:

• [ ] https://github.com/OCA/web/pull/886

[guewen]

@lreficent Do the tests pass in 9.0 and 10.0?

I'm struggling with the red zone test, which starts with:

    def test_buffer_zones_red(self):
        method = self.env.ref('ddmrp.adu_calculation_method_fixed')
        orderpointA = self.orderpointModel.create({
            'buffer_profile_id': self.buffer_profile_pur.id,
            'product_id': self.productA.id,
            'location_id': self.stock_location.id,
            'warehouse_id': self.warehouse.id,
            'product_min_qty': 0.0,
            'product_max_qty': 0.0,
            'qty_multiple': 0.0,
            'dlt': 10,
            'adu_calculation_method': method.id,
            'adu_fixed': 4
        })
        self.orderpointModel.cron_ddmrp_adu()

        self._check_red_zone(orderpointA, red_base_qty=20, red_safety_qty=10,
                             red_zone_qty=30)

Should be rather simple, the red zone is computed as:

    @api.multi
    @api.depends("dlt", "adu", "buffer_profile_id.lead_time_id.factor",
                 "buffer_profile_id.variability_id.factor",
                 "product_uom.rounding")
    def _compute_red_zone(self):
        for rec in self:
            rec.red_base_qty = float_round(
                rec.dlt * rec.adu * rec.buffer_profile_id.lead_time_id.factor,
                precision_rounding=rec.product_uom.rounding)
            rec.red_safety_qty = float_round(
                rec.red_base_qty * rec.buffer_profile_id.variability_id.factor,
                precision_rounding=rec.product_uom.rounding)
            rec.red_zone_qty = rec.red_base_qty + rec.red_safety_qty

But with the default data provided to the test, I have:

• dlt: 1.0
• adu: 4.0
• lead time factor: 0.5

Which gives a red base of 2.0 instead of 20. The other numbers the same. They all have a factor 10 less.

The issue lies in the dlt which is always equal to 1.0. The test creates the orderpoint with a dlt value of 10, but this field is a computed field, so it should not be written to.

The field is defined as:

    def _compute_dlt(self):
        for rec in self:
            if rec.buffer_profile_id.item_type == 'manufactured':
                bom = rec._get_manufactured_bom()
                rec.dlt = bom.dlt
            else:
                rec.dlt = rec.product_id.seller_ids and \
                          rec.product_id.seller_ids[0].delay or rec.lead_days

    dlt = fields.Float(string="Decoupled Lead Time (days)",
                       compute="_compute_dlt")

But it seems this compute method is never called, whatever I do the value of the dlt is always 1.0. I even put an exception inside the _compute_dlt method, nothing was raised.

Do you have an idea? Don't you have the same issue in 10.0?

[guewen]

I don't have anymore this issue... crazy. But anyway I had to set a lead_day in the test instead of the dlt

[guewen]

Another doubt regarding the tests, linking code from 9.0:

In https://github.com/Eficent/ddmrp/blob/0f172f6e552387185933f6621daad3ecc0829dfb/ddmrp/tests/test_ddmrp.py#L671

The on_hand_percent result is described as : On hand / Top of Green * 100. Top of green in the test is indeed 90. and the quantity on hand is 200. So the expected result would be 222.22

https://github.com/Eficent/ddmrp/blob/0f172f6e552387185933f6621daad3ecc0829dfb/ddmrp/tests/test_ddmrp.py#L723-L725

But in the method that computes on_hand_percent, it's On Hand / Top of Red * 100:

https://github.com/Eficent/ddmrp/blob/0f172f6e552387185933f6621daad3ecc0829dfb/ddmrp/models/stock_warehouse_orderpoint.py#L600-L605

Which gives a different result of course as Top of Red is 30, we get 666.66

@lreficent @jbeficent Is the test or the implementation lying? :)

[LoisRForgeFlow]

@guewen Trust the code, test are a little bit behind at the moment :sweat_smile: We have being implementing fixes and new features and didn't have time to update the test yet. Maybe after the next week's event we should do a mini-sprint to update them to make your life easier...

But in the method that computes on_hand_percent, it's On Hand / Top of Red * 100:

That's the good one!

[guewen]

@lreficent okay so I'll try to correct them (I have yet only 2 of them to fix), but as I'm far to be an expert in ddmrp, I hope I won't make them wrong :) Thanks!

[LoisRForgeFlow]

@guewen Great! BTW, we pushed today some changes to v10 and we're likely pushing a pair of addition tomorrow, you might want to rebase.