-
Notifications
You must be signed in to change notification settings - Fork 17
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.
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
Migration to version 11.0 #28
Comments
base_cron_exclusion PR#1156 |
mrp_mto_with_stock PR#255 |
there is no need to migrate |
@guewen I'm finishing the migration to v10, I'll create the v11 branch when ready. I expect it to be done today. |
@lreficent great! thanks, looking forward to start on v11 :) |
@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 :) |
@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 :) |
@guewen branch 11 available https://github.com/Eficent/ddmrp/tree/11.0 |
@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. The orderpoint is found from the procurement.order in a computed field: Action: In 11.0, the B. Override of Action: The original method has been moved to C. ProcurementOrder.add_to_net_flow_equation It's related to And 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 Action: Not sure yet. I removed it but... E. The orderpoint is found from the procurement orders. We have to find from where we can know it now. Action: from my research: there is no 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 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: Action: move to 'stock.scheduler.compute' |
Here is my progress so far: https://github.com/Eficent/ddmrp/compare/11.0...guewen:mig-ddmrp?expand=1 |
List of additional pull requests required: |
@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:
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? |
I don't have anymore this issue... crazy. |
Another doubt regarding the tests, linking code from 9.0: The But in the method that computes 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? :) |
@guewen Trust the code, test are a little bit behind at the moment 😅 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...
That's the good one! |
@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! |
@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. |
Moved to OCA/ddmrp#7
The text was updated successfully, but these errors were encountered: