diff options
| author | Nicolas Lempereur <nle@odoo.com> | 2017-03-15 17:11:16 +0100 |
|---|---|---|
| committer | Nicolas Lempereur <nle@odoo.com> | 2017-03-16 11:57:04 +0100 |
| commit | cc1c15327cddf2b512e01e0b7b31e2aa6a1d34fe (patch) | |
| tree | cfb1d840e285c63ec461fd450070dfdeb09ec142 /odoo | |
| parent | 27f656a23e92df52b2f78fdd7c31b8c045b1f39b (diff) | |
[FIX] expression: improve o2m with m2o integer
We sometimes use in Odoo a One2many field with an inverse Integer
field instead of a usual Many2one.
This allow for example in several instances to have a "Many2one" which
can be reference from several models, eg:
Model Ranking:
name = String field
res_id = Integer field
res_model = String field
Model Toy:
rank = One2many [inverse: Ranking -> res_id]
[domain: res_model == Toy]
Model Tool:
rank = One2many [inverse: Ranking -> res_id]
[domain: res_model == Tool]
This enable us to have a shared feature between otherwise unrelated models.
But there was several issue when searching on these One2many:
1) if the Integer Many2one was not stored (eg. it came from an inherits) on
the searched model, this could lead to an error.
2) when we searched:
- by IDs (rank in ['55']) with at least one id not respecting the domain
- by IDs with a negative operator
- with a negative operator on unfound string (rank != "no rank has this")
- with a false value (rank = False)
we would not apply the One2many domain (eg. res_model == Toy) and thus
possibly getting Toy 3 errenously because a Tool 3 was found without
the domain being applied.
This fix modify the search on One2many and for:
1. if the inverse is an Integer not stored field instead of Many2one
manage it.
2. if the field is an Integer field instead of Many2one and there is a
domain on the One2many: apply the domain on the inverse model found.
So only some search on One2many with a domain whose inverse field is an
Integer could be impacted.
This would also be nice to have for all One2many with a domain but the
probability of it being useful versus risk for performance is not judged
high enough.
opw-710508
Diffstat (limited to 'odoo')
| -rw-r--r-- | odoo/addons/test_new_api/ir.model.access.csv | 3 | ||||
| -rw-r--r-- | odoo/addons/test_new_api/models.py | 27 | ||||
| -rw-r--r-- | odoo/addons/test_new_api/tests/test_one2many.py | 67 | ||||
| -rw-r--r-- | odoo/osv/expression.py | 32 |
4 files changed, 118 insertions, 11 deletions
diff --git a/odoo/addons/test_new_api/ir.model.access.csv b/odoo/addons/test_new_api/ir.model.access.csv index 3862f4ce84f..0dd4efcda72 100644 --- a/odoo/addons/test_new_api/ir.model.access.csv +++ b/odoo/addons/test_new_api/ir.model.access.csv @@ -5,6 +5,9 @@ access_message,test_new_api_message,test_new_api.model_test_new_api_message,,1,1 access_emailmessage,test_new_api_emailmessage,test_new_api.model_test_new_api_emailmessage,,1,1,1,1 access_multi,test_new_api_multi,test_new_api.model_test_new_api_multi,,1,1,1,1 access_multi_line,test_new_api_multi_line,test_new_api.model_test_new_api_multi_line,,1,1,1,1 +access_creativework_edition,test_new_api_creativework_edition,model_test_new_api_creativework_edition,,1,1,1,1 +access_creativework_book,test_new_api_creativework_book,model_test_new_api_creativework_book,,1,1,1,1 +access_creativework_movie,test_new_api_creativework_movie,model_test_new_api_creativework_movie,,1,1,1,1 access_mixed,test_new_api_mixed,test_new_api.model_test_new_api_mixed,,1,1,1,1 access_domain_bool,access_domain_bool,model_domain_bool,,1,1,1,1 access_test_new_api_foo,access_test_new_api_foo,model_test_new_api_foo,,1,1,1,1 diff --git a/odoo/addons/test_new_api/models.py b/odoo/addons/test_new_api/models.py index 0d2054e32e5..81e24fb5b5b 100644 --- a/odoo/addons/test_new_api/models.py +++ b/odoo/addons/test_new_api/models.py @@ -199,6 +199,33 @@ class MultiLine(models.Model): partner = fields.Many2one('res.partner') +class Edition(models.Model): + _name = 'test_new_api.creativework.edition' + + name = fields.Char() + res_id = fields.Integer(required=True) + res_model_id = fields.Many2one('ir.model', required=True) + res_model = fields.Char(related='res_model_id.model', store=True) + + +class Book(models.Model): + _name = 'test_new_api.creativework.book' + + name = fields.Char() + editions = fields.One2many( + 'test_new_api.creativework.edition', 'res_id', domain=[('res_model', '=', _name)] + ) + + +class Movie(models.Model): + _name = 'test_new_api.creativework.movie' + + name = fields.Char() + editions = fields.One2many( + 'test_new_api.creativework.edition', 'res_id', domain=[('res_model', '=', _name)] + ) + + class MixedModel(models.Model): _name = 'test_new_api.mixed' diff --git a/odoo/addons/test_new_api/tests/test_one2many.py b/odoo/addons/test_new_api/tests/test_one2many.py index 38dee13ab54..9ade2c0fe19 100644 --- a/odoo/addons/test_new_api/tests/test_one2many.py +++ b/odoo/addons/test_new_api/tests/test_one2many.py @@ -10,6 +10,36 @@ class One2manyCase(TransactionCase): "name": "What is up?" }) + # data for One2many with inverse field Integer + self.Edition = self.env["test_new_api.creativework.edition"] + self.Book = self.env["test_new_api.creativework.book"] + self.Movie = self.env["test_new_api.creativework.movie"] + + book_model_id = self.env['ir.model'].search([('model', '=', self.Book._name)]).id + movie_model_id = self.env['ir.model'].search([('model', '=', self.Movie._name)]).id + + books_data = ( + ('Imaginary book', ()), + ('Another imaginary book', ()), + ('Nineteen Eighty Four', ('First edition', 'Fourth Edition')) + ) + + movies_data = ( + ('The Gold Rush', ('1925 (silent)', '1942')), + ('Imaginary movie', ()), + ('Another imaginary movie', ()) + ) + + for name, editions in books_data: + book_id = self.Book.create({'name': name}).id + for edition in editions: + self.Edition.create({'res_model_id': book_model_id, 'name': edition, 'res_id': book_id}) + + for name, editions in movies_data: + movie_id = self.Movie.create({'name': name}).id + for edition in editions: + self.Edition.create({'res_model_id': movie_model_id, 'name': edition, 'res_id': movie_id}) + def operations(self): """Run operations on o2m fields to check all works fine.""" # Check the lines first @@ -63,3 +93,40 @@ class One2manyCase(TransactionCase): """Check lines created with RPC style and added in one step""" self.multi.lines = [(0, 0, {'name': str(name)}) for name in range(10)] self.operations() + + def test_many2one_integer(self): + """Test several models one2many with same inverse Integer field""" + # utility function to convert records to tuples with id,name + t = lambda records: records.mapped(lambda r: (r.id, r.name)) + + books = self.Book.search([]) + books_with_edition = books.filtered(lambda r: r.editions) + movies = self.Movie.search([]) + movies_without_edition = movies.filtered(lambda r: not r.editions) + movies_with_edition = movies.filtered(lambda r: r.editions) + movie_editions = movies_with_edition.mapped('editions') + one_movie_edition = movie_editions[0] + + res_movies_without_edition = self.Movie.search([('editions', '=', False)]) + self.assertItemsEqual(t(res_movies_without_edition), t(movies_without_edition)) + + res_movies_with_edition = self.Movie.search([('editions', '!=', False)]) + self.assertItemsEqual(t(res_movies_with_edition), t(movies_with_edition)) + + res_books_with_movie_edition = self.Book.search([('editions', 'in', movie_editions.ids)]) + self.assertFalse(t(res_books_with_movie_edition)) + + res_books_without_movie_edition = self.Book.search([('editions', 'not in', movie_editions.ids)]) + self.assertItemsEqual(t(res_books_without_movie_edition), t(books_with_edition)) + + res_books_without_one_movie_edition = self.Book.search([('editions', 'not in', movie_editions[:1].ids)]) + self.assertItemsEqual(t(res_books_without_one_movie_edition), t(books_with_edition)) + + res_books_with_one_movie_edition_name = self.Book.search([('editions', '=', movie_editions[:1].name)]) + self.assertFalse(t(res_books_with_one_movie_edition_name)) + + res_books_without_one_movie_edition_name = self.Book.search([('editions', '!=', movie_editions[:1].name)]) + self.assertItemsEqual(t(res_books_without_one_movie_edition_name), t(books_with_edition)) + + res_movies_not_of_edition_name = self.Movie.search([('editions', '!=', one_movie_edition.name)]) + self.assertItemsEqual(t(res_movies_not_of_edition_name), t(movies.filtered(lambda r: one_movie_edition not in r.editions))) diff --git a/odoo/osv/expression.py b/odoo/osv/expression.py index da093dad323..eea14988eb7 100644 --- a/odoo/osv/expression.py +++ b/odoo/osv/expression.py @@ -920,19 +920,23 @@ class expression(object): elif field.type == 'one2many': call_null = True + domain = field.domain + if callable(domain): + domain = domain(model) + is_integer_m2o = comodel._fields[field.inverse_name].type == 'integer' if right is not False: if isinstance(right, basestring): op = {'!=': '=', 'not like': 'like', 'not ilike': 'ilike'}.get(operator, operator) - domain = field.domain - if callable(domain): - domain = domain(model) ids2 = [x[0] for x in comodel.name_search(right, domain or [], op, limit=None)] if ids2: operator = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in' - elif isinstance(right, collections.Iterable): - ids2 = right else: - ids2 = [right] + if isinstance(right, collections.Iterable): + ids2 = right + else: + ids2 = [right] + if ids2 and is_integer_m2o and domain: + ids2 = comodel.search([('id', 'in', ids2)] + domain).ids if not ids2: if operator in ['like', 'ilike', 'in', '=']: @@ -945,7 +949,9 @@ class expression(object): ids1 = select_from_where(cr, field.inverse_name, comodel._table, 'id', ids2, operator) else: recs = comodel.browse(ids2).sudo().with_context(prefetch_fields=False) - ids1 = recs.mapped(field.inverse_name).ids + ids1 = recs.mapped(field.inverse_name) + if not is_integer_m2o: + ids1 = ids1.ids if ids1: call_null = False o2m_op = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in' @@ -958,12 +964,16 @@ class expression(object): if call_null: o2m_op = 'in' if operator in NEGATIVE_TERM_OPERATORS else 'not in' # determine ids from field.inverse_name - if comodel._fields[field.inverse_name].store: + if comodel._fields[field.inverse_name].store and not (is_integer_m2o and domain): ids1 = select_distinct_from_where_not_null(cr, field.inverse_name, comodel._table) else: - domain = [(field.inverse_name, '!=', False)] - recs = comodel.search(domain).sudo().with_context(prefetch_fields=False) - ids1 = recs.mapped(field.inverse_name).ids + comodel_domain = [(field.inverse_name, '!=', False)] + if is_integer_m2o and domain: + comodel_domain += domain + recs = comodel.search(comodel_domain).sudo().with_context(prefetch_fields=False) + ids1 = recs.mapped(field.inverse_name) + if not is_integer_m2o: + ids1 = ids1.ids push(create_substitution_leaf(leaf, ('id', o2m_op, ids1), model)) elif field.type == 'many2many': |
