[FIX] sale_management: (revert) handle temp recs

[ethanrobv]

This PR reverts the commit
c6842f1

opw-3754297

[robodoo]

Pull request status dashboard.

[nle-odoo]

looks good to me, wouldn't this be equivalent of new_sol.order_id = False (which I would expect to update the order_line one2many linked to order_id) that was before the original commit? Here it seems a little complex to understand what we are doing to me (why not fetch order_line instead for example)

robodoo delegate+

[ethanrobv]

looks good to me, wouldn't this be equivalent of new_sol.order_id = False (which I would expect to update the order_line one2many linked to order_id) that was before the original commit? Here it seems a little complex to understand what we are doing to me (why not fetch order_line instead for example)

I tried it with the order_id = False (old way) and was seeing the same result, so maybe I am missing something here. I also tried to option.order_id.fetch('order_line') but it was also not solving the issue. I am generally a bit unsure of why this fix works or what exactly the root problem is here.

[nle-odoo]

yes, it's a little hard to understand, I've also tried this change (get back to original code):

diff --git a/addons/sale_management/models/sale_order_option.py b/addons/sale_management/models/sale_order_option.py
index bc9ea344d1a6..2d8d4f8db5dc 100644
--- a/addons/sale_management/models/sale_order_option.py
+++ b/addons/sale_management/models/sale_order_option.py
@@ -90,8 +90,7 @@ class SaleOrderOption(models.Model):
             new_sol._compute_price_unit()
             option.price_unit = new_sol.price_unit
             # Drop the temporary record from the cache
-            new_sol.invalidate_recordset(flush=False)
-            option.order_id.order_line.fetch(['id'])
+            new_sol.order_id = False
 
     @api.depends('product_id', 'uom_id', 'quantity')
     def _compute_discount(self):
@@ -104,8 +103,7 @@ class SaleOrderOption(models.Model):
             new_sol._compute_discount()
             option.discount = new_sol.discount
             # Drop the temporary record from the cache
-            new_sol.invalidate_recordset(flush=False)
-            option.order_id.order_line.fetch(['id'])
+            new_sol.order_id = False
 
     def _get_values_to_add_to_order(self):
         self.ensure_one()

and the test works

also with this change:

diff --git a/addons/sale_management/models/sale_order_option.py b/addons/sale_management/models/sale_order_option.py
index bc9ea344d1a6..d5adee90864b 100644
--- a/addons/sale_management/models/sale_order_option.py
+++ b/addons/sale_management/models/sale_order_option.py
@@ -89,9 +89,6 @@ class SaleOrderOption(models.Model):
             new_sol = self.env['sale.order.line'].new(values)
             new_sol._compute_price_unit()
             option.price_unit = new_sol.price_unit
-            # Drop the temporary record from the cache
-            new_sol.invalidate_recordset(flush=False)
-            option.order_id.order_line.fetch(['id'])
 
     @api.depends('product_id', 'uom_id', 'quantity')
     def _compute_discount(self):
@@ -103,9 +100,6 @@ class SaleOrderOption(models.Model):
             new_sol = self.env['sale.order.line'].new(values)
             new_sol._compute_discount()
             option.discount = new_sol.discount
-            # Drop the temporary record from the cache
-            new_sol.invalidate_recordset(flush=False)
-            option.order_id.order_line.fetch(['id'])
 
     def _get_values_to_add_to_order(self):
         self.ensure_one()

the test also works.

But if there is the invalidated_recordset (only from 17.0, it works in 16.0 up to saas-16.4) it doesn't work.

So is the original issue happening in 17.0? maybe we could just keep the current code (so revert the fix) if the issue from a change in ORM (or in business code) did not happen in 17.

as a side note: another things that seem to make the test work in saas-16.4 and below instead of invalidate_recordset is new_sol.orer_id.order_line -= new_sol (but not sure why it works and not new_sol.order_id = False, probably because it does an unlink which does other things).

I am generally a bit unsure of why this fix works or what exactly the root problem is here.

Checking the SQL, it seems that we will get the ID of sale order line from the database and update order_line, so since the record was never in database it makes sense it works.

[ethanrobv]

@nle-odoo Thanks a lot for your notes, I agree with your assessment that we should rollback the 17.0 community commit (but perhaps we want to keep the test added in enterprise? not sure...)

[ethanrobv]

@fw-bot ignore

[fw-bot]

Forward-port disabled.

[Feyensv]

Well, I don't mind going back to the old state of things, though it's strange that it broke 'without explanation' starting from this version 🤔

@robodoo r+

[Feyensv]

robodoo delegate+ too