From da48700b59da49cdc810eac31a8617fcaa0dc2a8 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Tue, 4 Oct 2022 09:09:46 +0000 Subject: [PATCH 1/5] [IMP] base: do not modify the cache when reading properties fields in batch Purpose ======= Do not modify the cache in order to improve the performance when we read in batch. Instead we create a new batched method "convert_to_read" that check existence in batch, and generate a dict with the result. Now, all the properties field checks (many2one existence, selection option still exists, tag value stiff exist, etc) and done in "convert_to_read". It means that doing `record.properties` won't do all those checks. Having the batched field fetch at the convert_to_record required too many changes for the scope of this task, so "record.properties" having the cache values unchecked is an acceptable tradeoff currently, hence we batch convert_to_read. Task-2980121 Part-of: odoo/odoo#101901 --- .../test_new_api/tests/test_properties.py | 24 +++-- odoo/fields.py | 95 +++++++++---------- odoo/models.py | 21 +++- 3 files changed, 80 insertions(+), 60 deletions(-) diff --git a/odoo/addons/test_new_api/tests/test_properties.py b/odoo/addons/test_new_api/tests/test_properties.py index 0bd1bc53507..eed97fbf02d 100644 --- a/odoo/addons/test_new_api/tests/test_properties.py +++ b/odoo/addons/test_new_api/tests/test_properties.py @@ -592,7 +592,7 @@ class PropertiesCase(TransactionCase): # 1 query to read the field # 1 query to read the definition # 2 queries to check if the many2one still exists / name_get - self.assertFalse(self.message_2.attributes[0]['value']) + self.assertFalse(self.message_2.read(['attributes'])[0]['attributes'][0]['value']) # remove the partner, and use the read method self.message_2.attributes = [{ @@ -844,9 +844,9 @@ class PropertiesCase(TransactionCase): # the value must remain in the database until the next write on the child self.assertEqual(self._get_sql_properties(message), {'my_tags': ['be', 'de']}) - + attributes = message.read(['attributes'])[0]['attributes'] self.assertEqual( - message.attributes[0]['value'], + attributes[0]['value'], ['be'], msg='The tag has been removed on the definition, should be removed when reading the child') self.assertEqual( @@ -854,7 +854,7 @@ class PropertiesCase(TransactionCase): [['be', 'BE', 1], ['fr', 'FR', 2], ['it', 'IT', 1]]) # next write on the child must update the value - message.attributes = message.attributes + message.attributes = message.read(['attributes'])[0]['attributes'] self.assertEqual(self._get_sql_properties(message), {'my_tags': ['be']}) @@ -890,7 +890,7 @@ class PropertiesCase(TransactionCase): 'comodel': 'test_new_api.partner', }] - with self.assertQueryCount(2): + with self.assertQueryCount(5): self.message_1.attributes = [ { "name": "moderator_partner_ids", @@ -900,11 +900,13 @@ class PropertiesCase(TransactionCase): "value": partners[:10].name_get(), } ] - self.assertEqual(self.message_1.attributes[0]['value'], partners[:10].ids) + attributes = self.message_1.read(['attributes'], load=None)[0]['attributes'] + self.assertEqual(attributes[0]['value'], partners[:10].ids) partners[:5].unlink() - with self.assertQueryCount(5): - self.assertEqual(self.message_1.attributes[0]['value'], partners[5:10].ids) + with self.assertQueryCount(4): + attributes = self.message_1.read(['attributes'], load=None)[0]['attributes'] + self.assertEqual(attributes[0]['value'], partners[5:10].ids) partners[5].unlink() with self.assertQueryCount(5): @@ -1045,6 +1047,12 @@ class PropertiesCase(TransactionCase): ] self.message_1.flush_recordset() + last_message_id = self.env['test_new_api.message'].search([], order="id DESC", limit=1).id + # based on batch optimization, _read_format should not crash on non existing records + values = self.env['test_new_api.message'].browse((self.message_1.id, last_message_id + 1))._read_format(['attributes']) + self.assertEqual(len(values), 1) + self.assertEqual(values[0]['id'], self.message_1.id) + def test_properties_field_change_definition(self): """Test the behavior of the field when changing the definition.""" diff --git a/odoo/fields.py b/odoo/fields.py index fd91d6d849e..cdfe7d271f1 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -3290,7 +3290,6 @@ class Properties(Field): assert isinstance(value, dict), f"Wrong type {value!r}" value = self._dict_to_list(value, definition) - self._parse_json_types(value, record.env) return value @@ -3312,10 +3311,22 @@ class Properties(Field): # }] # def convert_to_read(self, value, record, use_name_get=True): + return self.convert_to_read_multi([value], record, use_name_get)[0] + + def convert_to_read_multi(self, values, records, use_name_get=True): + assert len(values) == len(records) + + res_ids_per_model = self._get_res_ids_per_model(records, values, use_name_get) + # value is in record format + for value in values: + self._parse_json_types(value, records.env, res_ids_per_model) + if use_name_get: - self._add_display_name(value, record.env) - return value + for value in values: + self._add_display_name(value, records.env) + + return values def convert_to_write(self, value, record): """If we write a list on the child, update the definition record.""" @@ -3330,37 +3341,29 @@ class Properties(Field): self._add_display_name(value, record.env) return value - def read(self, records): + def _get_res_ids_per_model(self, records, values_list, use_name_get=True): """Read everything needed in batch for the given records. To retrieve relational properties names, or to check their existence, we need to do some SQL queries. To reduce the number of queries when we read - in batch, we put in cache everything needed before calling + in batch, we prefetch everything needed before calling convert_to_record / convert_to_read. - """ - definition_records_map = { - record: record[self.definition_record][self.definition_record_field] - for record in records - } + Return a dict {model: record_ids} that contains + the existing ids for each needed models. + """ # ids per model we need to fetch in batch to put in cache ids_per_model = defaultdict(OrderedSet) - records_cached_values = list(records.env.cache.get_values(records, self)) - - for record, record_values in zip(records, records_cached_values): - definition = definition_records_map.get(record) - if not record_values or not definition: - continue - for property_definition in definition: + for record, record_values in zip(records, values_list): + for property_definition in record_values: comodel = property_definition.get('comodel') type_ = property_definition.get('type') - name = property_definition.get('name') - if not comodel or type_ not in ('many2one', 'many2many') or name not in record_values: - continue - + property_value = property_definition.get('value') or [] default = property_definition.get('default') or [] - property_value = record_values[name] or [] + + if type_ not in ('many2one', 'many2many') or comodel not in records.env: + continue if type_ == 'many2one': default = [default] if default else [] @@ -3370,38 +3373,20 @@ class Properties(Field): ids_per_model[comodel].update(property_value) # check existence and pre-fetch in batch - existing_ids_per_model = {} + res_ids_per_model = {} for model, ids in ids_per_model.items(): recs = records.env[model].browse(ids).exists() - existing_ids_per_model[model] = set(recs.ids) - for record in recs: - # read a field to pre-fetch the recordset - try: - record.display_name - except AccessError: - pass + res_ids_per_model[model] = set(recs.ids) - # update the cache and remove non-existing ids - for record, record_values in zip(records, records_cached_values): - definition = definition_records_map.get(record) - if not record_values or not definition: - continue + if use_name_get: + for record in recs: + # read a field to pre-fetch the recordset + try: + record.display_name + except AccessError: + pass - for property_definition in definition: - comodel = property_definition.get('comodel') - type_ = property_definition.get('type') - name = property_definition.get('name') - if not comodel or type_ not in ('many2one', 'many2many') or not record_values.get(name): - continue - - property_value = record_values[name] - - if type_ == 'many2one': - record_values[name] = property_value if property_value in existing_ids_per_model[comodel] else False - else: - record_values[name] = [id_ for id_ in property_value if id_ in existing_ids_per_model[comodel]] - - records.env.cache.update(record, self, [record_values], check_dirty=False) + return res_ids_per_model def write(self, records, value): """Check if the properties definition has been changed. @@ -3591,7 +3576,7 @@ class Properties(Field): definition['name'] = str(uuid.uuid4()).replace('-', '')[:16] @classmethod - def _parse_json_types(cls, values_list, env): + def _parse_json_types(cls, values_list, env, res_ids_per_model): """Parse the value stored in the JSON. Check for records existence, if we removed a selection option, ... @@ -3633,6 +3618,9 @@ class Properties(Field): if not isinstance(property_value, int): raise ValueError(f'Wrong many2one value: {property_value!r}.') + if property_value not in res_ids_per_model[res_model]: + property_value = False + elif property_type == 'many2many' and property_value and res_model in env: if not is_list_of(property_value, int): raise ValueError(f'Wrong many2many value: {property_value!r}.') @@ -3641,6 +3629,11 @@ class Properties(Field): # remove duplicated value and preserve order property_value = list(dict.fromkeys(property_value)) + property_value = [ + id_ for id_ in property_value + if id_ in res_ids_per_model[res_model] + ] + property_definition['value'] = property_value @classmethod diff --git a/odoo/models.py b/odoo/models.py index a91e57d7d45..1213b3ab4d3 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3144,13 +3144,32 @@ class BaseModel(metaclass=MetaModel): The output format is the one expected from the `read` method, which uses this method as its implementation for formatting values. + For the properties fields, call convert_to_read_multi instead of convert_to_read + to prepare everything (record existences, display name, etc) in batch. + The current method is different from `read` because it retrieves its values from the cache without doing a query when it is avoidable. """ data = [(record, {'id': record._ids[0]}) for record in self] use_name_get = (load == '_classic_read') for name in fnames: - convert = self._fields[name].convert_to_read + field = self._fields[name] + if field.type == 'properties': + values_list = [] + records = [] + for record, vals in data: + try: + values_list.append(record[name]) + records.append(record.id) + except MissingError: + vals.clear() + + results = field.convert_to_read_multi(values_list, self.browse(records), use_name_get) + for record_read_vals, convert_result in zip(data, results): + record_read_vals[1][name] = convert_result + continue + + convert = field.convert_to_read for record, vals in data: # missing records have their vals empty if not vals: From 0583a5ddd0b0285b110fb393ef0212c4a7ccb5d4 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Tue, 13 Sep 2022 06:56:31 +0000 Subject: [PATCH 2/5] [IMP] base: search on properties fields Purpose ======= Allow to search properties fields. For now, the properties fields are meant to be used with the frontend top bar filter on views in a later commit, and so everything as been kept as simple as possible for that particular use case. Technical ========= Relational properties --------------------- We can not search on relational properties using the rec name, or using one of their fields. The following domains do not work ``` [('properties.partner_id', 'ilike', 'alice')] [('properties.partner_ids', 'ilike', 'alice')] [('properties.user_id.name', 'ilike', 'bob')] [('properties.user_ids.name', 'ilike', 'bob')] ``` Instead, we should give the ids ``` [('properties.partner_id', '=', 55)] [('properties.partner_ids', 'in', 55)] ``` IN operator ----------- The "in" operator is used to either search specific tags / many2many values, either doing a "OR like" condition on "non-array" value. It can be used if the left or the right side is not an array ``` [('properties.my_char', 'in', ['a', 'b'])] [('properties.my_tags', 'in', 'a')] ``` But it can't be used if both side of the condition are array, because it will require to check type in SQL (which is technically possible, but add extra complexity, so for now we keep the feature as simple as possible). So the following domain is not supported. ``` [('properties.my_tags', 'in', ['a', 'b'])] ``` Instead we should use the "OR" operator ``` ['|', ('properties.my_tags', 'in', 'a'), ('properties.my_tags', 'in', 'b')] ``` Tags / many2many - "=" operator ------------------------------- Tags and many2many have to be searched using the "in" operator. So the following domain will return unexpected results and raise a warning. ``` [('attributes.mytags', '=', ['a', 'b'])] ``` The reason is that we don't want to dynamically check the type in SQL, and so we need the "in" domain operator to know what to do in SQL. Task-2980121 Part-of: odoo/odoo#101901 --- .../test_new_api/tests/test_properties.py | 284 +++++++++++++++++- odoo/fields.py | 1 + odoo/models.py | 7 + odoo/osv/expression.py | 55 +++- 4 files changed, 344 insertions(+), 3 deletions(-) diff --git a/odoo/addons/test_new_api/tests/test_properties.py b/odoo/addons/test_new_api/tests/test_properties.py index eed97fbf02d..408d1c6b9ad 100644 --- a/odoo/addons/test_new_api/tests/test_properties.py +++ b/odoo/addons/test_new_api/tests/test_properties.py @@ -3,6 +3,7 @@ import json +import unittest from unittest.mock import patch from odoo import Command @@ -996,7 +997,7 @@ class PropertiesCase(TransactionCase): 'discussion': self.discussion_1.id, 'author': self.user.id, 'attributes': [{ - 'name': 'My Tags', + 'name': 'my_tags', 'type': 'many2many', 'comodel': 'test_new_api.multi.tag', 'value': tags.ids, @@ -1335,7 +1336,6 @@ class PropertiesCase(TransactionCase): } self.assertEqual(expected_properties, sql_properties) - @mute_logger('odoo.fields') def test_properties_field_security(self): """Check the access right related to the Properties fields.""" MultiTag = type(self.env['test_new_api.multi.tag']) @@ -1388,3 +1388,283 @@ class PropertiesCase(TransactionCase): value = self.env.cr.fetchone() self.assertTrue(value and value[0]) return value[0] + + +class PropertiesSearchCase(PropertiesCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.messages = cls.message_1 | cls.message_2 | cls.message_3 + cls.env['test_new_api.message'].search([('id', 'not in', cls.messages.ids)]).unlink() + + @mute_logger('odoo.fields') + def test_properties_field_search_boolean(self): + # search on boolean + self.message_1.attributes = [{ + 'name': 'myboolean', + 'type': 'boolean', + 'value': True, + 'definition_changed': True, + }] + self.message_2.attributes = {'myboolean': False} + messages = self.env['test_new_api.message'].search([('attributes.myboolean', '=', True)]) + self.assertEqual(messages, self.message_1) + messages = self.env['test_new_api.message'].search([('attributes.myboolean', '!=', False)]) + self.assertEqual(messages, self.message_1) + messages = self.env['test_new_api.message'].search([('attributes.myboolean', '=', False)]) + # message 2 has a falsy boolean properties + # message 3 doesn't have the properties (key in dict doesn't exist) + self.assertEqual(messages, self.message_2 | self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.myboolean', '!=', True)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + @mute_logger('odoo.fields') + def test_properties_field_search_char(self): + # search on text properties + self.message_1.attributes = [{ + 'name': 'mychar', + 'type': 'char', + 'value': 'Test', + 'definition_changed': True, + }] + self.message_2.attributes = {'mychar': 'TeSt'} + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', 'Test')]) + self.assertEqual(messages, self.message_1, "Should be able to search on a properties field") + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', '"Test"')]) + self.assertFalse(messages) + messages = self.env['test_new_api.message'].search([('attributes.mychar', 'ilike', 'test')]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mychar', 'not ilike', 'test')]) + self.assertFalse(messages) + messages = self.env['test_new_api.message'].search([('attributes.mychar', 'ilike', '"test"')]) + self.assertFalse(messages) + + for forbidden_char in '! ()"\'.': + searches = ( + f'mychar{forbidden_char}', + f'my{forbidden_char}char', + f'{forbidden_char}mychar', + ) + for search in searches: + with self.assertRaises(ValueError), self.assertQueryCount(0): + self.env['test_new_api.message'].search([(f'attributes.{search}', '=', 'Test')]) + + # search falsy properties + self.message_3.discussion = self.message_2.discussion + self.message_3.attributes = [{'name': 'mychar', 'value': False}] + self.assertEqual(self._get_sql_properties(self.message_3), {'mychar': False}) + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', False)]) + self.assertEqual(messages, self.message_3) + + # search falsy properties when the key doesn't exist in the dict + # message 2 properties is False, message 3 properties doesn't exist in database + self.message_2.attributes = [{'name': 'mychar', 'value': False}] + self.env.cr.execute( + "UPDATE test_new_api_message SET attributes = '{}' WHERE id = %s", + [self.message_3.id], + ) + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', False)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '!=', False)]) + self.assertEqual(messages, self.message_1) + + # message 1 property contain a string but is not falsy so it's not returned + messages = self.env['test_new_api.message'].search([('attributes.mychar', '!=', True)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', True)]) + self.assertEqual(messages, self.message_1) + + # message 3 is now null instead of being an empty dict + self.env.cr.execute( + "UPDATE test_new_api_message SET attributes = NULL WHERE id = %s", + [self.message_3.id], + ) + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', False)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '!=', False)]) + self.assertEqual(messages, self.message_1) + + @mute_logger('odoo.fields') + def test_properties_field_search_float(self): + # search on float + self.message_1.attributes = [{ + 'name': 'myfloat', + 'type': 'float', + 'value': 3.14, + 'definition_changed': True, + }] + self.message_2.attributes = {'myfloat': 5.55} + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '>', 4.4)]) + self.assertEqual(messages, self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '<', 4.4)]) + self.assertEqual(messages, self.message_1) + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '>', 1.1)]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '<=', 1.1)]) + self.assertFalse(messages) + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '=', 3.14)]) + self.assertEqual(messages, self.message_1) + + @mute_logger('odoo.fields') + def test_properties_field_search_integer(self): + # search on integer + self.messages.discussion = self.discussion_1 + self.message_1.attributes = [{ + 'name': 'myint', + 'type': 'integer', + 'value': 33, + 'definition_changed': True, + }] + self.message_2.attributes = {'myint': 111} + self.message_3.attributes = {'myint': -2} + + messages = self.env['test_new_api.message'].search([('attributes.myint', '>', 4)]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.myint', '<', 4)]) + self.assertEqual(messages, self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.myint', '=', 111)]) + self.assertEqual(messages, self.message_2) + # search on the JSONified value (operator "->>") + messages = self.env['test_new_api.message'].search([('attributes.myint', 'ilike', '1')]) + self.assertEqual(messages, self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.myint', 'not ilike', '1')]) + self.assertEqual(messages, self.message_1 | self.message_3) + + @mute_logger('odoo.fields') + def test_properties_field_search_many2many(self): + self.messages.discussion = self.discussion_1 + partners = self.env['res.partner'].create([{'name': 'A'}, {'name': 'B'}, {'name': 'C'}]) + self.message_1.attributes = [{ + 'name': 'mymany2many', + 'type': 'many2many', + 'comodel': 'res.partner', + 'value': partners.ids, + 'definition_changed': True, + }] + self.message_2.attributes = {'mymany2many': [partners[1].id]} + self.message_3.attributes = {'mymany2many': [partners[2].id]} + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'in', partners[0].id)]) + self.assertEqual(messages, self.message_1) + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'in', partners[1].id)]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'in', partners[2].id)]) + self.assertEqual(messages, self.message_1 | self.message_3) + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'not in', partners[0].id)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + # IN operator (not supported on many2many and return weird results) + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'in', [partners[0].id, partners[1].id])]) + self.assertEqual(messages, self.message_2) # should be self.message_1 | self.message_2 + + @mute_logger('odoo.fields') + def test_properties_field_search_many2one(self): + # many2one are just like integer + self.messages.discussion = self.discussion_1 + self.message_1.attributes = [{ + 'name': 'mypartner', + 'type': 'integer', + 'value': self.partner.id, + 'definition_changed': True, + }] + self.message_2.attributes = {'mypartner': self.partner_2.id} + self.message_3.attributes = {'mypartner': False} + + messages = self.env['test_new_api.message'].search( + [('attributes.mypartner', 'in', [self.partner.id, self.partner_2.id])]) + self.assertEqual(messages, self.message_1 | self.message_2) + + messages = self.env['test_new_api.message'].search( + [('attributes.mypartner', 'not in', [self.partner.id, self.partner_2.id])]) + self.assertEqual(messages, self.message_3) + + messages = self.env['test_new_api.message'].search( + [('attributes.mypartner', 'ilike', self.partner.display_name)]) + self.assertFalse(messages, "The ilike on relational properties is not supported") + + @mute_logger('odoo.fields') + def test_properties_field_search_tags(self): + self.messages.discussion = self.discussion_1 + self.message_1.attributes = [{ + 'name': 'mytags', + 'type': 'tags', + 'value': ['a', 'b'], + 'tags': [['a', 'A', 1], ['b', 'B', 2], ['aa', 'AA', 3]], + 'definition_changed': True, + }] + self.message_2.attributes = {'mytags': ['b']} + self.message_3.attributes = {'mytags': ['aa']} + + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', 'a')]) + self.assertEqual(messages, self.message_1) + # the search is done on the JSONified value (operator "->>") + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'ilike', 'a')]) + self.assertEqual(messages, self.message_1 | self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'not ilike', 'a')]) + self.assertEqual(messages, self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', 'b')]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', 'aa')]) + self.assertEqual(messages, self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'not in', 'b')]) + self.assertEqual(messages, self.message_3) + # the search is done on the JSONified value (operator "->>") + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'ilike', '["aa"]')]) + self.assertEqual(messages, self.message_3) + + # IN operator on array + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', [])]) + self.assertFalse(messages) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'not in', [])]) + self.assertEqual(messages, self.message_1 | self.message_2 | self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', ['a', 'b'])]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', ['b', 'a'])]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', ['aa'])]) + self.assertEqual(messages, self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', ['aa', 'b'])]) + self.assertEqual(messages, self.message_3 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'not in', ['a', 'b'])]) + self.assertEqual(messages, self.message_3) + + @mute_logger('odoo.fields') + def test_properties_field_search_unaccent(self): + if not self.registry.has_unaccent: + # To enable unaccent feature: + # CREATE EXTENSION unaccent; + raise unittest.SkipTest("unaccent not enabled") + + Model = self.env['test_new_api.message'] + (self.message_1 | self.message_2).discussion = self.discussion_1 + # search on text properties + self.message_1.attributes = [{ + 'name': 'mychar', + 'type': 'char', + 'value': 'Hélène', + 'definition_changed': True, + }] + self.message_2.attributes = {'mychar': 'Helene'} + + result = Model.search([('attributes.mychar', 'ilike', 'Helene')]) + self.assertEqual(self.message_1 | self.message_2, result) + + result = Model.search([('attributes.mychar', 'ilike', 'hélène')]) + self.assertEqual(self.message_1 | self.message_2, result) + + result = Model.search([('attributes.mychar', 'not ilike', 'Helene')]) + self.assertNotIn(self.message_1, result) + self.assertNotIn(self.message_2, result) + + result = Model.search([('attributes.mychar', 'not ilike', 'hélène')]) + self.assertNotIn(self.message_1, result) + self.assertNotIn(self.message_2, result) diff --git a/odoo/fields.py b/odoo/fields.py index cdfe7d271f1..7b4fc7f6bc4 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -3196,6 +3196,7 @@ class Properties(Field): column_type = ('jsonb', 'jsonb') copy = False prefetch = False + unaccent = True write_sequence = 10 # because it must be written after the definition field # the field is computed editable by design (see the compute method below) diff --git a/odoo/models.py b/odoo/models.py index 1213b3ab4d3..49ce8712492 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -68,6 +68,7 @@ from .tools.translate import _, _lt _logger = logging.getLogger(__name__) _unlink = logging.getLogger(__name__ + '.unlink') +regex_alphanumeric = re.compile(r'^[a-z0-9_]+$') regex_order = re.compile(r'^(\s*([a-z0-9:_]+|"[a-z0-9:_]+")(\.id)?(\s+(desc|asc))?\s*(,|$))+(?', '>=', '<', '<=', 'in', 'not in', 'like', 'ilike', 'not like', 'not ilike'): + raise ValueError(f"Wrong search operator {operator!r}") + property_name = path[1] + check_property_field_value_name(property_name) + + if (isinstance(right, bool) or right is None) and operator in ('=', '!='): + # check for boolean value but also for key existence + if right: + # inverse the condition + right = False + operator = '!=' if operator == '=' else '=' + + sql_path = f'"{alias}"."{field.name}"' + key_existence_expr = "" + if operator == '=': # property == False + key_existence_expr = ( + f"OR ({sql_path} IS NULL) " + f"OR NOT ({sql_path} ? '{property_name}')" + ) + + expr = f"(({sql_path} -> '{property_name}') {operator} '%s' {key_existence_expr})" + push_result(expr, [right]) + + else: + if 'like' in operator: + right = f'%{pycompat.to_text(right)}%' + unaccent = self._unaccent(field) + else: + unaccent = lambda x: x + + inverse = '' + arrow = '->' # raw value + if operator in ('in', 'not in'): + if operator == 'not in': + inverse = 'NOT' + if isinstance(right, (list, tuple)): + operator = '<@' + else: + operator = '@>' + right = json.dumps(right) + elif isinstance(right, str): + arrow = '->>' # JSONified value + else: + right = json.dumps(right) + + sql_path = f""""{alias}"."{field.name}" {arrow} '{property_name}'""" + expr = f"""({inverse} ({unaccent(sql_path)}) {operator} ({unaccent('%s')}))""" + push_result(expr, [right]) + # ---------------------------------------- # PATH SPOTTED # -> many2one or one2many with _auto_join: From defa39062131e62a6e2c0992df1df6f989fdf1cc Mon Sep 17 00:00:00 2001 From: std-odoo Date: Thu, 15 Sep 2022 06:43:01 +0000 Subject: [PATCH 3/5] [IMP] web: allow to search properties fields using the top search bar Purpose ======= Add the properties fields in the search bar and allow to search properties fields values. Technical ========= If the "active_id" is in the context, show only the properties of the given definition record. Otherwise, show all properties of all definition records. Because fetching the properties definitions cost a SQL query and a RPC call, the properties definition are fetched only if we unfold the properties field (so it has no performance impact until we ask explicitly to search a property). Task-2980121 Part-of: odoo/odoo#101901 Co-authored-by: Polymorphe57 --- .../static/src/search/search_arch_parser.js | 18 +- .../src/search/search_bar/search_bar.js | 253 +++++++----- .../src/search/search_bar/search_bar.xml | 8 +- addons/web/static/src/search/search_model.js | 147 +++++-- .../static/tests/search/search_bar_tests.js | 359 ++++++++++++++++++ .../static/tests/search/search_model_tests.js | 33 ++ odoo/addons/base/models/ir_ui_view.py | 2 +- 7 files changed, 697 insertions(+), 123 deletions(-) diff --git a/addons/web/static/src/search/search_arch_parser.js b/addons/web/static/src/search/search_arch_parser.js index e9a499d0cad..fbb270d982a 100644 --- a/addons/web/static/src/search/search_arch_parser.js +++ b/addons/web/static/src/search/search_arch_parser.js @@ -127,15 +127,16 @@ export class SearchArchParser extends XMLParser { } if (node.hasAttribute("name")) { const name = node.getAttribute("name"); + const fieldType = this.fields[name].type; preField.fieldName = name; - preField.fieldType = this.fields[name].type; - if (name in this.searchDefaults) { + preField.fieldType = fieldType; + if (fieldType !== "properties" && name in this.searchDefaults) { preField.isDefault = true; let value = this.searchDefaults[name]; value = Array.isArray(value) ? value[0] : value; let operator = preField.operator; if (!operator) { - let type = preField.fieldType; + let type = fieldType; if (node.hasAttribute("widget")) { type = node.getAttribute("widget"); } @@ -149,8 +150,7 @@ export class SearchArchParser extends XMLParser { } } preField.defaultRank = -10; - const { fieldType, fieldName } = preField; - const { selection, context, relation } = this.fields[fieldName]; + const { selection, context, relation } = this.fields[name]; preField.defaultAutocompleteValue = { label: `${value}`, operator, value }; if (fieldType === "selection") { const option = selection.find((sel) => sel[0] === value); @@ -160,9 +160,11 @@ export class SearchArchParser extends XMLParser { preField.defaultAutocompleteValue.label = option[1]; } else if (fieldType === "many2one") { this.labels.push((orm) => { - return orm.call(relation, "name_get", [value], { context }).then((results) => { - preField.defaultAutocompleteValue.label = results[0][1]; - }); + return orm + .call(relation, "name_get", [value], { context }) + .then((results) => { + preField.defaultAutocompleteValue.label = results[0][1]; + }); }); } } diff --git a/addons/web/static/src/search/search_bar/search_bar.js b/addons/web/static/src/search/search_bar/search_bar.js index bd5e166d52e..d1e5e86c544 100644 --- a/addons/web/static/src/search/search_bar/search_bar.js +++ b/addons/web/static/src/search/search_bar/search_bar.js @@ -10,14 +10,15 @@ import { fuzzyTest } from "@web/core/utils/search"; import { Component, useExternalListener, useRef, useState } from "@odoo/owl"; const parsers = registry.category("parsers"); -const CHAR_FIELDS = ["char", "html", "many2many", "many2one", "one2many", "text"]; +const CHAR_FIELDS = ["char", "html", "many2many", "many2one", "one2many", "text", "properties"]; +const FOLDABLE_TYPES = ["properties", "many2one", "many2many"]; let nextItemId = 1; export class SearchBar extends Component { setup() { this.fields = this.env.searchModel.searchViewFields; - this.searchItems = this.env.searchModel.getSearchItems((f) => f.type === "field"); + this.searchItemsFields = this.env.searchModel.getSearchItems((f) => f.type === "field"); this.root = useRef("root"); // core state @@ -49,9 +50,13 @@ export class SearchBar extends Component { useExternalListener(window, "keydown", this.onWindowKeydown); } - //--------------------------------------------------------------------- - // Private - //--------------------------------------------------------------------- + /** + * @param {number} id + * @param {Object} + */ + getSearchItem(id) { + return this.env.searchModel.searchItems[id]; + } /** * @param {Object} [options={}] @@ -70,8 +75,11 @@ export class SearchBar extends Component { const tasks = []; for (const id of expanded) { - if (!subItems[id]) { - tasks.push({ id, prom: this.computeSubItems(id, query) }); + const searchItem = this.getSearchItem(id); + if (searchItem.type === "field" && searchItem.fieldType === "properties") { + tasks.push({ id, prom: this.getSearchItemsProperties(searchItem) }); + } else if (!subItems[id]) { + tasks.push({ id, prom: this.computeSubItems(searchItem, query) }); } } @@ -98,88 +106,144 @@ export class SearchBar extends Component { return; } - for (const searchItem of this.searchItems) { - const field = this.fields[searchItem.fieldName]; - const type = field.type === "reference" ? "char" : field.type; - /** @todo do something with respect to localization (rtl) */ - const preposition = this.env._t(["date", "datetime"].includes(type) ? "at" : "for"); - - if (["selection", "boolean"].includes(type)) { - const options = field.selection || [ - [true, this.env._t("Yes")], - [false, this.env._t("No")], - ]; - for (const [value, label] of options) { - if (fuzzyTest(trimmedQuery.toLowerCase(), label.toLowerCase())) { - this.items.push({ - id: nextItemId++, - searchItemDescription: searchItem.description, - preposition, - searchItemId: searchItem.id, - label, - /** @todo check if searchItem.operator is fine (here and elsewhere) */ - operator: searchItem.operator || "=", - value, - }); - } - } - continue; - } - - const parser = parsers.contains(type) ? parsers.get(type) : (str) => str; - let value; - try { - switch (type) { - case "date": { - value = serializeDate(parser(trimmedQuery)); - break; - } - case "datetime": { - value = serializeDateTime(parser(trimmedQuery)); - break; - } - case "many2one": { - value = trimmedQuery; - break; - } - default: { - value = parser(trimmedQuery); - } - } - } catch { - continue; - } - - const item = { - id: nextItemId++, - searchItemDescription: searchItem.description, - preposition, - searchItemId: searchItem.id, - label: this.state.query, - operator: searchItem.operator || (CHAR_FIELDS.includes(type) ? "ilike" : "="), - value, - }; - - if (type === "many2one") { - item.isParent = true; - item.isExpanded = this.state.expanded.includes(item.searchItemId); - } - - this.items.push(item); - - if (item.isExpanded) { - this.items.push(...this.subItems[searchItem.id]); - } + for (const searchItem of this.searchItemsFields) { + this.items.push(...this.getItems(searchItem, trimmedQuery)); } } /** - * @param {number} searchItemId + * @param {Object} searchItem + * @param {string} trimmedQuery + * @returns {Object[]} + */ + getItems(searchItem, trimmedQuery) { + const items = []; + + const isFieldProperty = searchItem.type === "field_property"; + + const { type } = isFieldProperty + ? searchItem.propertyFieldDefinition + : this.fields[searchItem.fieldName]; + const fieldType = type === "reference" ? "char" : type; + + /** @todo do something with respect to localization (rtl) */ + let preposition = this.env._t(["date", "datetime"].includes(fieldType) ? "at" : "for"); + + if ((isFieldProperty && FOLDABLE_TYPES.includes(fieldType)) || fieldType === "properties") { + // Do not chose preposition for foldable properties + // or the properties item itself + preposition = null; + } + + if (["selection", "boolean", "tags"].includes(fieldType)) { + const booleanOptions = [ + [true, this.env._t("Yes")], + [false, this.env._t("No")], + ]; + let options; + if (isFieldProperty) { + const { selection, tags } = searchItem.propertyFieldDefinition || {}; + options = selection || tags || booleanOptions; + } else { + options = this.fields[searchItem.fieldName].selection || booleanOptions; + } + for (const [value, label] of options) { + if (fuzzyTest(trimmedQuery.toLowerCase(), label.toLowerCase())) { + items.push({ + id: nextItemId++, + searchItemDescription: searchItem.description, + preposition, + searchItemId: searchItem.id, + label, + /** @todo check if searchItem.operator is fine (here and elsewhere) */ + operator: searchItem.operator || "=", + value, + isFieldProperty, + }); + } + } + return items; + } + + const parser = parsers.contains(fieldType) ? parsers.get(fieldType) : (str) => str; + let value; + try { + switch (fieldType) { + case "date": { + value = serializeDate(parser(trimmedQuery)); + break; + } + case "datetime": { + value = serializeDateTime(parser(trimmedQuery)); + break; + } + case "many2one": { + value = trimmedQuery; + break; + } + default: { + value = parser(trimmedQuery); + } + } + } catch { + return []; + } + + const item = { + id: nextItemId++, + searchItemDescription: searchItem.description, + preposition, + searchItemId: searchItem.id, + label: this.state.query, + operator: searchItem.operator || (CHAR_FIELDS.includes(fieldType) ? "ilike" : "="), + value, + isFieldProperty, + }; + + if (isFieldProperty) { + item.isParent = FOLDABLE_TYPES.includes(fieldType); + item.unselectable = FOLDABLE_TYPES.includes(fieldType); + item.propertyItemId = searchItem.propertyItemId; + } else if (fieldType === "properties") { + item.isParent = true; + item.unselectable = true; + } else if (fieldType === "many2one") { + item.isParent = true; + } + + if (item.isParent) { + item.isExpanded = this.state.expanded.includes(item.searchItemId); + } + + items.push(item); + + if (item.isExpanded) { + if (searchItem.type === "field" && searchItem.fieldType === "properties") { + for (const subItem of this.subItems[searchItem.id]) { + items.push(...this.getItems(subItem, trimmedQuery)); + } + } else { + items.push(...this.subItems[searchItem.id]); + } + } + + return items; + } + + /** + * @param {Object} searchItem + * @returns {Object[]} + */ + getSearchItemsProperties(searchItem) { + return this.env.searchModel.getSearchItemsProperties(searchItem); + } + + /** + * @param {Object} searchItem * @param {string} query * @returns {Object[]} */ - async computeSubItems(searchItemId, query) { - const searchItem = this.searchItems.find((i) => i.id === searchItemId); + async computeSubItems(searchItem, query) { const field = this.fields[searchItem.fieldName]; let domain = []; if (searchItem.domain) { @@ -189,7 +253,12 @@ export class SearchBar extends Component { // Pass } } - const options = await this.orm.call(field.relation, "name_search", [], { + const relation = + searchItem.type === "field_property" + ? searchItem.propertyFieldDefinition.comodel + : field.relation; + + const options = await this.orm.call(relation, "name_search", [], { args: domain, context: { ...this.env.searchModel.globalContext, ...field.context }, limit: 8, @@ -202,7 +271,7 @@ export class SearchBar extends Component { subItems.push({ id: nextItemId++, isChild: true, - searchItemId, + searchItemId: searchItem.id, value, label, operator, @@ -212,7 +281,7 @@ export class SearchBar extends Component { subItems.push({ id: nextItemId++, isChild: true, - searchItemId, + searchItemId: searchItem.id, label: this.env._t("(no result)"), unselectable: true, }); @@ -251,6 +320,15 @@ export class SearchBar extends Component { * @param {Object} item */ selectItem(item) { + const searchItem = this.getSearchItem(item.searchItemId); + if ( + (searchItem.type === "field" && searchItem.fieldType === "properties") || + (searchItem.type === "field_property" && item.unselectable) + ) { + this.toggleItem(item, !item.isExpanded); + return; + } + if (!item.unselectable) { const { searchItemId, label, operator, value } = item; this.env.searchModel.addAutoCompletionValues(searchItemId, { label, operator, value }); @@ -373,6 +451,11 @@ export class SearchBar extends Component { focusedIndex = this.items.findIndex( (item) => item.isParent && item.searchItemId === focusedItem.searchItemId ); + } else if (focusedItem && focusedItem.isFieldProperty) { + ev.preventDefault(); + focusedIndex = this.items.findIndex( + (item) => item.isParent && item.searchItemId === focusedItem.propertyItemId + ); } else if (ev.target.selectionStart === 0) { // focus rightmost facet if any. this.focusFacet(); diff --git a/addons/web/static/src/search/search_bar/search_bar.xml b/addons/web/static/src/search/search_bar/search_bar.xml index 974603a1dcf..ca7b6f1a7d8 100644 --- a/addons/web/static/src/search/search_bar/search_bar.xml +++ b/addons/web/static/src/search/search_bar/search_bar.xml @@ -55,20 +55,18 @@ t-on-mousemove="() => this.onItemMousemove(item_index)" > - - + - - Search : - + Search : diff --git a/addons/web/static/src/search/search_model.js b/addons/web/static/src/search/search_model.js index 27bcaf15af5..83ebdc51d34 100644 --- a/addons/web/static/src/search/search_model.js +++ b/addons/web/static/src/search/search_model.js @@ -121,15 +121,8 @@ function arraytoMap(array) { * @param {Object} target */ function execute(op, source, target) { - const { - query, - nextId, - nextGroupId, - nextGroupNumber, - searchItems, - searchPanelInfo, - sections, - } = source; + const { query, nextId, nextGroupId, nextGroupNumber, searchItems, searchPanelInfo, sections } = + source; target.nextGroupId = nextGroupId; target.nextGroupNumber = nextGroupNumber; @@ -518,7 +511,7 @@ export class SearchModel extends EventBus { */ addAutoCompletionValues(searchItemId, autocompleteValue) { const searchItem = this.searchItems[searchItemId]; - if (searchItem.type !== "field") { + if (!["field", "field_property"].includes(searchItem.type)) { return; } const { label, value, operator } = autocompleteValue; @@ -715,9 +708,11 @@ export class SearchModel extends EventBus { return searchItem.comparison; } const { dateFilterId, comparisonOptionId } = searchItem; - const { fieldName, fieldType, description: dateFilterDescription } = this.searchItems[ - dateFilterId - ]; + const { + fieldName, + fieldType, + description: dateFilterDescription, + } = this.searchItems[dateFilterId]; const selectedGeneratorIds = this._getSelectedGeneratorIds(dateFilterId); // compute range and range description const { domain: range, description: rangeDescription } = constructDateDomain( @@ -727,16 +722,14 @@ export class SearchModel extends EventBus { selectedGeneratorIds ); // compute comparisonRange and comparisonRange description - const { - domain: comparisonRange, - description: comparisonRangeDescription, - } = constructDateDomain( - this.referenceMoment, - fieldName, - fieldType, - selectedGeneratorIds, - comparisonOptionId - ); + const { domain: comparisonRange, description: comparisonRangeDescription } = + constructDateDomain( + this.referenceMoment, + fieldName, + fieldType, + selectedGeneratorIds, + comparisonOptionId + ); return { comparisonId: comparisonOptionId, fieldName, @@ -854,6 +847,7 @@ export class SearchModel extends EventBus { switch (searchItem.type) { case "dateFilter": case "dateGroupBy": + case "field_property": case "field": { return; } @@ -941,10 +935,104 @@ export class SearchModel extends EventBus { this._notify(); } + /** + * Generate the searchItems corresponding to the properties. + * @param {Object} searchItem + * @returns {Object[]} + */ + async getSearchItemsProperties(searchItem) { + if (searchItem.type !== "field" || searchItem.fieldType !== "properties") { + return []; + } + const field = this.searchViewFields[searchItem.fieldName]; + const definitionRecord = field.definition_record; + const definitionRecordModel = this.searchViewFields[definitionRecord].relation; + const definitionRecordField = field.definition_record_field; + + const result = await this._fetchPropertiesDefinition( + definitionRecordModel, + definitionRecordField + ); + + const searchItemIds = new Set(); + const existingFieldProperties = {}; + for (const item of Object.values(this.searchItems)) { + if (item.type === "field_property" && item.propertyItemId === searchItem.id) { + existingFieldProperties[item.propertyFieldDefinition.name] = item; + } + } + + for (const { definitionRecordId, definitionRecordName, definitions } of result) { + for (const definition of definitions) { + const existingSearchItem = existingFieldProperties[definition.name]; + if (existingSearchItem) { + // already in the list, can happen if we unfold the properties field + // open a form view, edit the property and then go back to the search view + // the label of the property might have been changed + existingSearchItem.description = `${definition.string} (${definitionRecordName})`; + searchItemIds.add(existingSearchItem.id); + continue; + } + const id = this.nextId++; + const newSearchItem = { + id, + type: "field_property", + fieldName: searchItem.fieldName, + propertyDomain: [definitionRecord, "=", definitionRecordId], + propertyFieldDefinition: definition, + propertyItemId: searchItem.id, + description: `${definition.string} (${definitionRecordName})`, + groupId: this.nextGroupId++, + }; + if (["many2many", "tags"].includes(definition.type)) { + newSearchItem.operator = "in"; + } + this.searchItems[id] = newSearchItem; + searchItemIds.add(id); + } + } + + return this.getSearchItems((searchItem) => searchItemIds.has(searchItem.id)); + } + //-------------------------------------------------------------------------- // Private methods //-------------------------------------------------------------------------- + /** + * Fetch the properties definitions. + * + * @param {string} definitionRecordModel + * @param {string} definitionRecordField + * @return {Object[]} A list of objects of the form + * { + * definitionRecordId: + * definitionRecordName: + * definitions: + * } + */ + async _fetchPropertiesDefinition(definitionRecordModel, definitionRecordField) { + const domain = []; + if (this.context.active_id) { + // assume the active id is the definition record + // and show only its properties + domain.push(["id", "=", this.context.active_id]); + } + + const result = await this.orm.webSearchRead(definitionRecordModel, domain, [ + "display_name", + definitionRecordField, + ]); + + return result.records.map((values) => { + return { + definitionRecordId: values.id, + definitionRecordName: values.display_name, + definitions: values[definitionRecordField], + }; + }); + } + /** * Activate the default favorite (if any) or all default filters. */ @@ -1170,6 +1258,9 @@ export class SearchModel extends EventBus { * for some reason. */ _enrichItem(searchItem) { + if (searchItem.type === "field" && searchItem.fieldType === "properties") { + return { ...searchItem }; + } const queryElements = this.query.filter( (queryElem) => queryElem.searchItemId === searchItem.id ); @@ -1206,6 +1297,7 @@ export class SearchModel extends EventBus { ); break; case "field": + case "field_property": enrichSearchItem.autocompleteValues = queryElements.map( (queryElem) => queryElem.autocompleteValue ); @@ -1465,6 +1557,7 @@ export class SearchModel extends EventBus { for (const activeItem of group.activeItems) { const searchItem = this.searchItems[activeItem.searchItemId]; switch (searchItem.type) { + case "field_property": case "field": { type = "field"; title = searchItem.description; @@ -1541,8 +1634,13 @@ export class SearchModel extends EventBus { self: label.trim(), raw_value: value, }); - } else { + } else if (field.type === "field") { domain = [[field.fieldName, operator, value]]; + } else if (field.type === "field_property") { + domain = [ + field.propertyDomain, + [`${field.fieldName}.${field.propertyFieldDefinition.name}`, operator, value], + ]; } return new Domain(domain); }); @@ -1878,6 +1976,7 @@ export class SearchModel extends EventBus { const { searchItemId } = activeItem; const searchItem = this.searchItems[searchItemId]; switch (searchItem.type) { + case "field_property": case "field": { return this._getFieldDomain(searchItem, activeItem.autocompletValues); } diff --git a/addons/web/static/tests/search/search_bar_tests.js b/addons/web/static/tests/search/search_bar_tests.js index cc3ac129d8c..a4f7281112d 100644 --- a/addons/web/static/tests/search/search_bar_tests.js +++ b/addons/web/static/tests/search/search_bar_tests.js @@ -39,6 +39,12 @@ QUnit.module("Search", (hooks) => { foo: { string: "Foo", type: "char" }, bool: { string: "Bool", type: "boolean" }, company: { string: "Company", type: "many2one", relation: "partner" }, + properties: { + string: "Properties", + type: "properties", + definition_record: "bar", + definition_record_field: "child_properties", + }, }, records: [ { @@ -1054,4 +1060,357 @@ QUnit.module("Search", (hooks) => { await editSearch(target, "F"); await triggerEvent(target, ".o_searchview input", "keydown", { key: "ArrowRight" }); }); + + QUnit.test("search a property", async function (assert) { + assert.expect(57); + + async function mockRPC(_, { method, model, kwargs }) { + if ( + method === "web_search_read" && + model === "partner" && + kwargs.fields[0] === "display_name" && + kwargs.fields[1] === "child_properties" + ) { + const definition1 = [ + { + type: "many2one", + string: "My Partner", + name: "my_partner", + comodel: "partner", + }, + { + type: "many2many", + string: "My Partners", + name: "my_partners", + comodel: "partner", + }, + { + type: "selection", + string: "My Selection", + name: "my_selection", + selection: [ + ["a", "A"], + ["b", "B"], + ["c", "C"], + ["aa", "AA"], + ], + }, + { + type: "tags", + string: "My Tags", + name: "my_tags", + tags: [ + ["a", "A", 1], + ["b", "B", 5], + ["c", "C", 3], + ["aa", "AA", 2], + ], + }, + ]; + + const definition2 = [ + { + type: "char", + string: "My Text", + name: "my_text", + }, + ]; + + return { + records: [ + { id: 1, display_name: "Bar 1", child_properties: definition1 }, + { id: 2, display_name: "Bar 2", child_properties: definition2 }, + ], + }; + } else if (method === "name_search" && model === "partner" && kwargs.name === "Bo") { + return [ + [5, "Bob"], + [6, "Bobby"], + ]; + } else if (method === "name_search" && model === "partner" && kwargs.name === "Ali") { + return [ + [9, "Alice"], + [10, "Alicia"], + ]; + } + } + + const controlPanel = await makeWithSearch({ + serverData, + resModel: "partner", + Component: ControlPanel, + searchMenuTypes: [], + searchViewId: false, + searchViewArch: ` + + + + `, + mockRPC, + }); + + // expand the properties field + await editSearch(target, "a"); + + await click(target.querySelector(".o_expand")); + + let items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 8); + assert.strictEqual(items[0].innerText, "Search Properties"); + assert.strictEqual(items[1].innerText, "My Partner (Bar 1)"); + assert.strictEqual(items[3].innerText, "My Selection (Bar 1) for: A"); + assert.strictEqual(items[4].innerText, "My Selection (Bar 1) for: AA"); + assert.strictEqual(items[5].innerText, "My Tags (Bar 1) for: A"); + assert.strictEqual(items[6].innerText, "My Tags (Bar 1) for: AA"); + assert.strictEqual(items[7].innerText, "My Text (Bar 2) for: a"); + + // click again on the expand icon to hide the properties + await click(target.querySelector(".o_expand")); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 1, "Should have hidden the properties"); + + // search for a partner, and expand the many2many property + await editSearch(target, "Bo"); + await click(target.querySelector(".o_expand")); + await click(target.querySelector("li:nth-child(3) .o_expand")); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 6); + assert.strictEqual(items[3].innerText, "Bob"); + assert.strictEqual(items[4].innerText, "Bobby"); + + // fold all the properties (included the search result) + await click(target.querySelector(".o_expand")); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 1, "Should have folded the properties"); + + // unfold all the properties but fold the search result + await click(target.querySelector(".o_expand")); + await click(target.querySelector("li:nth-child(3) .o_expand")); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 4, "Should have unfolded the properties"); + assert.strictEqual(items[1].innerText, "My Partner (Bar 1)"); + assert.strictEqual(items[2].innerText, "My Partners (Bar 1)"); + assert.strictEqual(items[3].innerText, "My Text (Bar 2) for: Bo"); + + // select Bobby + await click(target.querySelector("li:nth-child(3) .o_expand")); + await click(target.querySelector(".o_searchview_input_container li:nth-child(5)")); + assert.deepEqual(getDomain(controlPanel), [ + "&", + ["bar", "=", 1], + ["properties.my_partners", "in", 6], + ]); + + // expand the selection properties + await click(target.querySelector(".o_control_panel")); + await editSearch(target, "a"); + await click(target.querySelector(".o_expand")); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 8, "Should have unfolded the selection properties"); + assert.strictEqual(items[3].innerText, "My Selection (Bar 1) for: A"); + assert.strictEqual(items[4].innerText, "My Selection (Bar 1) for: AA"); + + // select the selection option "AA" + await click(target.querySelector(".o_searchview_input_container li:nth-child(5)")); + let expectedDomain = [ + "&", + "&", + ["bar", "=", 1], + ["properties.my_partners", "in", 6], + "&", + ["bar", "=", 1], + ["properties.my_selection", "=", "aa"], + ]; + assert.deepEqual(getDomain(controlPanel), expectedDomain); + + // select the selection option "A" + await click(target.querySelector(".o_control_panel")); + await editSearch(target, "a"); + await click(target.querySelector(".o_expand")); + await click(target.querySelector(".o_searchview_input_container li:nth-child(4)")); + expectedDomain = [ + "&", + "&", + ["bar", "=", 1], + ["properties.my_partners", "in", 6], + "|", + "&", + ["bar", "=", 1], + ["properties.my_selection", "=", "aa"], + "&", + ["bar", "=", 1], + ["properties.my_selection", "=", "a"], + ]; + assert.deepEqual(getDomain(controlPanel), expectedDomain); + + // reset the search + await click(target.querySelector(".o_facet_remove")); + await click(target.querySelector(".o_facet_remove")); + + // search a many2one value + await click(target.querySelector(".o_control_panel")); + await editSearch(target, "Ali"); + await click(target.querySelector(".o_expand")); + await click(target.querySelector("li:nth-child(2) .o_expand")); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 6, "Should show the search result"); + assert.strictEqual(items[2].innerText, "Alice"); + assert.strictEqual(items[3].innerText, "Alicia"); + await click(target.querySelector(".o_searchview_input_container li:nth-child(4)")); + expectedDomain = ["&", ["bar", "=", 1], ["properties.my_partner", "=", 10]]; + assert.deepEqual(getDomain(controlPanel), expectedDomain); + + // search a tag value + await click(target.querySelector(".o_control_panel")); + await editSearch(target, "A"); + await click(target.querySelector(".o_expand")); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 8, "Should show the search result"); + assert.strictEqual(items[5].innerText, "My Tags (Bar 1) for: A"); + assert.strictEqual(items[6].innerText, "My Tags (Bar 1) for: AA"); + + await click(target.querySelector(".o_searchview_input_container li:nth-child(7)")); + expectedDomain = [ + "&", + "&", + ["bar", "=", 1], + ["properties.my_partner", "=", 10], + "&", + ["bar", "=", 1], + ["properties.my_tags", "in", "aa"], + ]; + assert.deepEqual(getDomain(controlPanel), expectedDomain); + // add the tag "B" + await click(target.querySelector(".o_control_panel")); + await editSearch(target, "B"); + await click(target.querySelector(".o_expand")); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 6, "Should show the search result"); + assert.strictEqual(items[4].innerText, "My Tags (Bar 1) for: B"); + await click(target.querySelector(".o_searchview_input_container li:nth-child(5)")); + expectedDomain = [ + "&", + "&", + ["bar", "=", 1], + ["properties.my_partner", "=", 10], + "|", + "&", + ["bar", "=", 1], + ["properties.my_tags", "in", "aa"], + "&", + ["bar", "=", 1], + ["properties.my_tags", "in", "b"], + ]; + assert.deepEqual(getDomain(controlPanel), expectedDomain); + + // try to click on the many2one properties without unfolding + // it should not add the domain, but unfold the item + await editSearch(target, "Bobby"); + await click(target.querySelector(".o_expand")); + await click(target.querySelector(".o_searchview_input_container li:nth-child(2)")); + assert.deepEqual(getDomain(controlPanel), expectedDomain); + items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 5, "Should have unfold the many2one"); + assert.strictEqual(items[2].innerText, "(no result)", "Should have unfold the many2one"); + + // test the navigation with keyboard + await editSearch(target, "Bo"); + const focusedItem = () => { + return target.querySelector(".o_menu_item.focus"); + }; + assert.strictEqual(focusedItem().innerText, "Search Properties"); + // unfold the properties field + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowRight" }); + assert.strictEqual(focusedItem().innerText, "Search Properties"); + assert.ok(focusedItem().querySelector(".fa-caret-down")); + // move on the many2one property + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowRight" }); + assert.strictEqual(focusedItem().innerText, "My Partner (Bar 1)"); + assert.ok(focusedItem().querySelector(".fa-caret-right")); + // move on the many2many property + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowDown" }); + assert.strictEqual(focusedItem().innerText, "My Partners (Bar 1)"); + assert.ok(focusedItem().querySelector(".fa-caret-right")); + // move on the many2one property again + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowUp" }); + assert.strictEqual(focusedItem().innerText, "My Partner (Bar 1)"); + assert.ok(focusedItem().querySelector(".fa-caret-right")); + // unfold the many2one + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowRight" }); + assert.strictEqual(focusedItem().innerText, "My Partner (Bar 1)"); + assert.ok(focusedItem().querySelector(".fa-caret-down")); + // select the first many2one + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowRight" }); + assert.strictEqual(focusedItem().innerText, "Bob"); + // go up on the parent + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowLeft" }); + assert.strictEqual(focusedItem().innerText, "My Partner (Bar 1)"); + assert.ok(focusedItem().querySelector(".fa-caret-down")); + // fold the parent + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowLeft" }); + assert.strictEqual(focusedItem().innerText, "My Partner (Bar 1)"); + assert.ok(focusedItem().querySelector(".fa-caret-right")); + // go up on the properties field + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowLeft" }); + assert.strictEqual(focusedItem().innerText, "Search Properties"); + assert.ok(focusedItem().querySelector(".fa-caret-down")); + // fold the properties field + await triggerEvent(document.activeElement, null, "keydown", { key: "ArrowLeft" }); + assert.strictEqual(focusedItem().innerText, "Search Properties"); + assert.ok(focusedItem().querySelector(".fa-caret-right")); + }); + + QUnit.test("search a property: definition record id in the context", async function (assert) { + assert.expect(3); + + async function mockRPC(route, { method, model, args, kwargs }) { + if ( + method === "web_search_read" && + model === "partner" && + kwargs.fields[0] === "display_name" && + kwargs.fields[1] === "child_properties" + ) { + assert.deepEqual( + kwargs.domain, + [["id", "=", 2]], + "Should search only the active parent properties" + ); + + const definition2 = [ + { + type: "char", + string: "My Text", + name: "my_text", + }, + ]; + + return { + records: [{ id: 2, display_name: "Bar 2", child_properties: definition2 }], + }; + } + } + + await makeWithSearch({ + serverData, + resModel: "partner", + Component: ControlPanel, + searchMenuTypes: [], + searchViewId: false, + searchViewArch: ` + + + + `, + mockRPC, + context: { active_id: 2 }, + }); + + await click(target.querySelector(".o_control_panel")); + await editSearch(target, "a"); + await click(target.querySelector(".o_expand")); + + const items = target.querySelectorAll(".o_searchview_input_container li"); + assert.strictEqual(items.length, 2, "Should show the search result"); + assert.strictEqual(items[1].innerText, "My Text (Bar 2) for: a"); + }); }); diff --git a/addons/web/static/tests/search/search_model_tests.js b/addons/web/static/tests/search/search_model_tests.js index d7b45266379..10fd547c0fa 100644 --- a/addons/web/static/tests/search/search_model_tests.js +++ b/addons/web/static/tests/search/search_model_tests.js @@ -49,6 +49,12 @@ QUnit.module("Search", (hooks) => { date_field: { string: "Date", type: "date", store: true, sortable: true }, float_field: { string: "Float", type: "float" }, bar: { string: "Bar", type: "many2one", relation: "partner" }, + properties: { + string: "Properties", + type: "properties", + definition_record: "bar", + definition_record_field: "child_properties", + }, }, records: [], }, @@ -958,4 +964,31 @@ QUnit.module("Search", (hooks) => { model.toggleSearchItem(1); assert.deepEqual(model.domain, [["date_deadline", "<", "2021-09-17"]]); }); + + QUnit.test( + "a field of type 'properties' should not be accepted as a search_default", + async function (assert) { + const searchViewArch = ` + + + + `; + + const model = await makeSearchModel({ + serverData, + searchViewArch, + context: { + search_default_properties: true, + }, + }); + assert.deepEqual(sanitizeSearchItems(model), [ + { + description: "Properties", + fieldName: "properties", + fieldType: "properties", + type: "field", + }, + ]); + } + ); }); diff --git a/odoo/addons/base/models/ir_ui_view.py b/odoo/addons/base/models/ir_ui_view.py index 0494c0adc6c..67d48ab45b0 100644 --- a/odoo/addons/base/models/ir_ui_view.py +++ b/odoo/addons/base/models/ir_ui_view.py @@ -2744,7 +2744,7 @@ class Model(models.AbstractModel): :rtype: list """ return [ - 'change_default', 'context', 'currency_field', 'definition_record', 'digits', 'domain', 'group_operator', 'groups', + 'change_default', 'context', 'currency_field', 'definition_record', 'definition_record_field', 'digits', 'domain', 'group_operator', 'groups', 'help', 'name', 'readonly', 'related', 'relation', 'relation_field', 'required', 'searchable', 'selection', 'size', 'sortable', 'store', 'string', 'translate', 'trim', 'type', ] From 1c2f29fc56868a9a27898270a8160bca5974b7e2 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Wed, 28 Sep 2022 09:57:30 +0000 Subject: [PATCH 4/5] [IMP] base: allow to order record by properties fields values Purpose ======= Allow to sort records by properties fields values. Task-2980121 Part-of: odoo/odoo#101901 --- .../test_new_api/tests/test_properties.py | 87 ++++++++++++++++++- odoo/fields.py | 2 + odoo/models.py | 10 ++- 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/odoo/addons/test_new_api/tests/test_properties.py b/odoo/addons/test_new_api/tests/test_properties.py index 408d1c6b9ad..36fac7f0f6f 100644 --- a/odoo/addons/test_new_api/tests/test_properties.py +++ b/odoo/addons/test_new_api/tests/test_properties.py @@ -7,7 +7,8 @@ import unittest from unittest.mock import patch from odoo import Command -from odoo.exceptions import AccessError + +from odoo.exceptions import AccessError, UserError from odoo.tests.common import Form, TransactionCase, users from odoo.tools import mute_logger @@ -1336,6 +1337,7 @@ class PropertiesCase(TransactionCase): } self.assertEqual(expected_properties, sql_properties) + @mute_logger('odoo.fields') def test_properties_field_security(self): """Check the access right related to the Properties fields.""" MultiTag = type(self.env['test_new_api.multi.tag']) @@ -1668,3 +1670,86 @@ class PropertiesSearchCase(PropertiesCase): result = Model.search([('attributes.mychar', 'not ilike', 'hélène')]) self.assertNotIn(self.message_1, result) self.assertNotIn(self.message_2, result) + + @mute_logger('odoo.fields') + def test_properties_field_search_orderby_string(self): + """Test that we can order record by properties string values.""" + (self.message_1 | self.message_2 | self.message_3).discussion = self.discussion_1 + self.message_1.attributes = [{ + 'name': 'mychar', + 'type': 'char', + 'value': 'BB', + 'definition_changed': True, + }] + self.message_2.attributes = {'mychar': 'AA'} + self.message_3.attributes = {'mychar': 'CC'} + + self.env.flush_all() + + result = self.env['test_new_api.message'].search( + domain=[['attributes.mychar', '!=', False]], + order='attributes.mychar ASC') + self.assertEqual(result[0], self.message_2) + self.assertEqual(result[1], self.message_1) + self.assertEqual(result[2], self.message_3) + + result = self.env['test_new_api.message'].search( + domain=[['attributes.mychar', '!=', False]], + order='attributes.mychar DESC') + self.assertEqual(result[0], self.message_3) + self.assertEqual(result[1], self.message_1) + self.assertEqual(result[2], self.message_2) + + @mute_logger('odoo.fields') + def test_properties_field_search_orderby_integer(self): + """Test that we can order record by properties integer values.""" + (self.message_1 | self.message_2 | self.message_3).discussion = self.discussion_1 + self.message_1.attributes = [{ + 'name': 'myinteger', + 'type': 'integer', + 'value': 22, + 'definition_changed': True, + }] + self.message_2.attributes = {'myinteger': 111} + self.message_3.attributes = {'myinteger': 33} + + self.env.flush_all() + + result = self.env['test_new_api.message'].search( + domain=[['attributes.myinteger', '!=', False]], + order='attributes.myinteger ASC') + self.assertEqual(result[0], self.message_1) + self.assertEqual(result[1], self.message_3) + self.assertEqual(result[2], self.message_2) + + result = self.env['test_new_api.message'].search( + domain=[['attributes.myinteger', '!=', False]], + order='attributes.myinteger DESC') + self.assertEqual(result[0], self.message_2) + self.assertEqual(result[1], self.message_3) + self.assertEqual(result[2], self.message_1) + + @mute_logger('odoo.fields') + def test_properties_field_search_orderby_injection(self): + """Check the restriction on the property name.""" + self.message_1.attributes = [{ + 'name': 'myinteger', + 'type': 'integer', + 'value': 22, + 'definition_changed': True, + }] + + for c in '! ()"\'.': + orders = ( + f'attributes.myinteger{c} ASC', + f'attributes.my{c}integer ASC', + f'attribut{c}es.myinteger ASC', + ) + + if c == ' ': + # allow multiple spaces after the property name + orders = orders[1:] + + for order in orders: + with self.assertRaises(UserError), self.assertQueryCount(0): + self.env['test_new_api.message'].search(domain=[], order=order) diff --git a/odoo/fields.py b/odoo/fields.py index 7b4fc7f6bc4..1b86947ae40 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -27,6 +27,7 @@ from psycopg2.sql import SQL, Identifier from difflib import get_close_matches from hashlib import sha256 +from .models import check_property_field_value_name from .tools import ( float_repr, float_round, float_compare, float_is_zero, human_size, pg_varchar, ustr, OrderedSet, pycompat, sql, date_utils, unique, @@ -3419,6 +3420,7 @@ class Properties(Field): ] for definition in value: definition.pop('definition_changed', None) + check_property_field_value_name(definition.get('name', '')) # update the properties definition on the container container = records[self.definition_record] diff --git a/odoo/models.py b/odoo/models.py index 49ce8712492..f8a9887ec29 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -69,7 +69,7 @@ _logger = logging.getLogger(__name__) _unlink = logging.getLogger(__name__ + '.unlink') regex_alphanumeric = re.compile(r'^[a-z0-9_]+$') -regex_order = re.compile(r'^(\s*([a-z0-9:_]+|"[a-z0-9:_]+")(\.id)?(\s+(desc|asc))?\s*(,|$))+(? '{property_name}')" order_by_elements.append("%s %s" % (qualifield_name, order_direction)) else: _logger.warning("Model %r cannot be sorted on field %r (not a column)", self._name, order_field) From 590beec44768889a7df0ddb166763772bb8722f6 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Mon, 13 Feb 2023 07:20:55 +0000 Subject: [PATCH 5/5] [IMP] crm, project: allow to search properties Purpose ======= Allow to search properties. Task-2980121 Part-of: odoo/odoo#101901 --- addons/crm/views/crm_lead_views.xml | 2 ++ addons/project/views/project_sharing_views.xml | 1 + addons/project/views/project_views.xml | 3 +++ 3 files changed, 6 insertions(+) diff --git a/addons/crm/views/crm_lead_views.xml b/addons/crm/views/crm_lead_views.xml index 0429305398d..b4fa6400b87 100644 --- a/addons/crm/views/crm_lead_views.xml +++ b/addons/crm/views/crm_lead_views.xml @@ -683,6 +683,7 @@ + + 999 + diff --git a/addons/project/views/project_views.xml b/addons/project/views/project_views.xml index da7304276b2..603c8febdae 100644 --- a/addons/project/views/project_views.xml +++ b/addons/project/views/project_views.xml @@ -80,6 +80,9 @@ primary + + +