14.0 Prevent cart changes when a transaction has been started

editable_cart.patch

diff --git addons/website_sale/controllers/main.py addons/website_sale/controllers/main.py
index b5bdc26c9a3..e24977212c4 100644
--- addons/website_sale/controllers/main.py
+++ addons/website_sale/controllers/main.py
@@ -384,7 +384,7 @@ class WebsiteSale(http.Controller):
         revive: Revival method when abandoned cart. Can be 'merge' or 'squash'
         """
         order = request.website.sale_get_order()
-        if order and order.state != 'draft':
+        if order and not order.is_cart_editable:
             request.session['sale_order_id'] = None
             order = request.website.sale_get_order()
         values = {}
@@ -425,7 +425,7 @@ class WebsiteSale(http.Controller):
     def cart_update(self, product_id, add_qty=1, set_qty=0, **kw):
         """This route is called when adding a product to cart (no options)."""
         sale_order = request.website.sale_get_order(force_create=True)
-        if sale_order.state != 'draft':
+        if not sale_order.is_cart_editable:
             request.session['sale_order_id'] = None
             sale_order = request.website.sale_get_order(force_create=True)
 
@@ -459,7 +459,7 @@ class WebsiteSale(http.Controller):
             - When adding a product to cart on the same page (without redirection).
         """
         order = request.website.sale_get_order(force_create=1)
-        if order.state != 'draft':
+        if not order.is_cart_editable:
             request.website.sale_reset()
             if kw.get('force_create'):
                 order = request.website.sale_get_order(force_create=1)
diff --git addons/website_sale/models/res_partner.py addons/website_sale/models/res_partner.py
index 5ef92cf6730..e52bdf7bdb5 100644
--- addons/website_sale/models/res_partner.py
+++ addons/website_sale/models/res_partner.py
@@ -16,10 +16,12 @@ class ResPartner(models.Model):
             is_public = any(u._is_public() for u in partner.with_context(active_test=False).user_ids)
             website = ir_http.get_request_website()
             if website and not is_public:
-                partner.last_website_so_id = SaleOrder.search([
+                last_website_so_id = SaleOrder.search([
                     ('partner_id', '=', partner.id),
                     ('website_id', '=', website.id),
-                    ('state', '=', 'draft'),
+                    ('is_cart_editable', '=', 'draft'),
                 ], order='write_date desc', limit=1)
+
+                partner.last_website_so_id = last_website_so_id if last_website_so_id.is_cart_editable else SaleOrder
             else:
                 partner.last_website_so_id = SaleOrder  # Not in a website context or public User
diff --git addons/website_sale/models/sale_order.py addons/website_sale/models/sale_order.py
index 092c7c81167..857ba892719 100644
--- addons/website_sale/models/sale_order.py
+++ addons/website_sale/models/sale_order.py
@@ -25,6 +25,7 @@ class SaleOrder(models.Model):
     cart_quantity = fields.Integer(compute='_compute_cart_info', string='Cart Quantity')
     only_services = fields.Boolean(compute='_compute_cart_info', string='Only Services')
     is_abandoned_cart = fields.Boolean('Abandoned Cart', compute='_compute_abandoned_cart', search='_search_abandoned_cart')
+    is_cart_editable = fields.Boolean('Is cart editable', compute='_compute_is_cart_editable')
     cart_recovery_email_sent = fields.Boolean('Cart recovery email already sent')
     website_id = fields.Many2one('website', string='Website', readonly=True,
                                  help='Website through which this order was placed.')
@@ -61,6 +62,19 @@ class SaleOrder(models.Model):
             else:
                 order.is_abandoned_cart = False
 
+    @api.depends('state')
+    def _compute_is_cart_editable(self):
+        """ Compute if a cart can be edited by customers.
+            We consider a cart to editable if it is in draft mode and have no pending/done
+            payments transaction linked to it (c.f: commit message for more info).
+        """
+        for cart in self:
+            paid_tx_count = self.env['payment.transaction'].search_count([
+                ('sale_order_ids', '=', cart.id),
+                ('state', 'in', ['pending', 'done']),
+            ])
+            cart.is_cart_editable = cart.state == 'draft' and paid_tx_count == 0
+
     def _search_abandoned_cart(self, operator, value):
         website_ids = self.env['website'].search_read(fields=['id', 'cart_abandoned_delay', 'partner_id'])
         deadlines = [[
diff --git addons/website_sale_product_configurator/controllers/main.py addons/website_sale_product_configurator/controllers/main.py
index ae6ddc5edee..c331b59ebbe 100644
--- addons/website_sale_product_configurator/controllers/main.py
+++ addons/website_sale_product_configurator/controllers/main.py
@@ -52,7 +52,7 @@ class WebsiteSale(main.WebsiteSale):
             request.website = request.website.with_context(lang=lang)
 
         order = request.website.sale_get_order(force_create=True)
-        if order.state != 'draft':
+        if not order.is_cart_editable:
             request.session['sale_order_id'] = None
             order = request.website.sale_get_order(force_create=True)
 

Just sharing my experience in writing business code 😉

('is_cart_editable', '=', 'draft') => boolean field
search_count If you don't care about the actual count, it is better to do a search with limit=1 for perfs
cart.state == 'draft' and paid_tx_count == 0 do not compute useless search in a loop if state is draft

search_count If you don't care about the actual count, it is better to do a search with limit=1 for perfs

Wrong: https://twitter.com/Xof/status/1413542818673577987

search_count If you don't care about the actual count, it is better to do a search with limit=1 for perfs

Wrong: https://twitter.com/Xof/status/1413542818673577987

LIMIT can cause very strange planning issues in PostgreSQL

While that is certainly true, I have never encountered such a case (in my limited experience) and every time I tested it, it was faster to use the limit=1
state is not indexed (by default at least), so it will be walking the entire table without the limit, won't it?

In a query plan, you can see 2 estimated costs. the first one is the cost to reach the first record, the second one the cost to reach all records. limit=1 cause PostgreSQL to choose the query plan based on the first cost. The assumption here is "The first record I'll reach is the good one", which can be False, especially when conditions on linked tables are involved.

Regarding index on state, you search for 'pending', 'done' states, this should be 90% of the table. Whatever the number of records, PG estimates it's lighter not to use index (index=search in index then look in table=cost of index + cost of look in table).