From 2875b94e036f024dcd37b72560fcaf349df27656 Mon Sep 17 00:00:00 2001
From: Adrian Torres <adt@odoo.com>
Date: Fri, 6 Apr 2018 07:45:25 +0200
Subject: [PATCH] [FIX] osv.expression: normalize domains in combine

Before this rev:

* Take 2 domains that contain at least two leaves, and at least one of
them must use the implicit `&` operator

e.g.:
    d1 = [('so_line', 'in', [91]), ('amount', '<=', 0.0)]
    d2 = ['&', ('so_line', 'in', []), ('project_id', '!=', False)]

* Perform osv.expression.OR() between both domains

Expected result:
    d3 = ['|', d1, d2]

Actual result (after normalization):
    d3 = ['&', '|', d1, d2]

This is because, since the `&` is implicit for the first domain, when we
OR it, we give it an explicit `|` operator, so when we pass this domain
through the normalize_domain function, d1 no longer contains an implicit
`&` operator but instead the implicit operator is the one between d1 and
d2, therefore giving us a completely wrong domain.

The `combine` function states that it only accepts normalized domains,
however neither the OR nor AND functions do, this leads to a lot of
developers putting non-normalized domains into these functions, and
there's no error checking or anything that obviously indicates that the
domain is incorrect, so we might as well normalize all domains being
passed since it's already pretty optimized.
---
 odoo/addons/base/tests/test_expression.py | 11 +++++++++++
 odoo/osv/expression.py                    |  7 ++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/odoo/addons/base/tests/test_expression.py b/odoo/addons/base/tests/test_expression.py
index e18e0b1240e..5800bad26d0 100644
--- a/odoo/addons/base/tests/test_expression.py
+++ b/odoo/addons/base/tests/test_expression.py
@@ -611,6 +611,17 @@ class TestExpression(TransactionCase):
         not_be = Partner.with_context(lang='fr_FR').search([('country_id', '!=', 'Belgique')])
         self.assertNotIn(agrolait, not_be)
 
+    def test_or_with_implicit_and(self):
+        # Check that when using expression.OR on a list of domains with at least one
+        # implicit '&' the returned domain is the expected result.
+        # from #24038
+        d1 = [('foo', '=', 1), ('bar', '=', 1)]
+        d2 = ['&', ('foo', '=', 2), ('bar', '=', 2)]
+
+        expected = ['|', '&', ('foo', '=', 1), ('bar', '=', 1),
+                         '&', ('foo', '=', 2), ('bar', '=', 2)]
+        self.assertEqual(expression.OR([d1, d2]), expected)
+
 
 class TestAutoJoin(TransactionCase):
 
diff --git a/odoo/osv/expression.py b/odoo/osv/expression.py
index 9e2bb7228e2..3e4a278ce5d 100644
--- a/odoo/osv/expression.py
+++ b/odoo/osv/expression.py
@@ -232,8 +232,9 @@ def is_false(model, domain):
 
 def combine(operator, unit, zero, domains):
     """Returns a new domain expression where all domain components from ``domains``
-       have been added together using the binary operator ``operator``. The given
-       domains must be normalized.
+       have been added together using the binary operator ``operator``.
+
+       It is guaranteed to return a normalized domain.
 
        :param unit: the identity element of the domains "set" with regard to the operation
                     performed by ``operator``, i.e the domain component ``i`` which, when
@@ -255,7 +256,7 @@ def combine(operator, unit, zero, domains):
         if domain == zero:
             return zero
         if domain:
-            result += domain
+            result += normalize_domain(domain)
             count += 1
     result = [operator] * (count - 1) + result
     return result