diff --git a/addons/web/static/src/views/fields/properties/kanban_properties_field.js b/addons/web/static/src/views/fields/properties/kanban_properties_field.js new file mode 100644 index 00000000000..9f1fc265d80 --- /dev/null +++ b/addons/web/static/src/views/fields/properties/kanban_properties_field.js @@ -0,0 +1,15 @@ +/** @odoo-module **/ + +import { registry } from "@web/core/registry"; +import { PropertiesField } from "./properties_field"; + +export class KanbanPropertiesField extends PropertiesField { + async _checkDefinitionAccess() { + // can not edit properties definition in the kanban view + this.state.canChangeDefinition = false; + } +} + +KanbanPropertiesField.template = "web.KanbanPropertiesField"; + +registry.category("fields").add("kanban.properties", KanbanPropertiesField); diff --git a/addons/web/static/src/views/fields/properties/kanban_properties_field.scss b/addons/web/static/src/views/fields/properties/kanban_properties_field.scss new file mode 100644 index 00000000000..35dbc965a04 --- /dev/null +++ b/addons/web/static/src/views/fields/properties/kanban_properties_field.scss @@ -0,0 +1,3 @@ +.o_kanban_property_field { + max-width: 300px; +} diff --git a/addons/web/static/src/views/fields/properties/kanban_properties_field.xml b/addons/web/static/src/views/fields/properties/kanban_properties_field.xml new file mode 100644 index 00000000000..2d412a85f41 --- /dev/null +++ b/addons/web/static/src/views/fields/properties/kanban_properties_field.xml @@ -0,0 +1,29 @@ + + + +
+
+
+ +
+
+
+
+
diff --git a/addons/web/static/src/views/fields/properties/properties_field.js b/addons/web/static/src/views/fields/properties/properties_field.js index 16974e31d00..e24fa581f73 100644 --- a/addons/web/static/src/views/fields/properties/properties_field.js +++ b/addons/web/static/src/views/fields/properties/properties_field.js @@ -154,9 +154,10 @@ export class PropertiesField extends Component { propertiesValues[targetIndex] = propertiesValues[propertyIndex]; propertiesValues[propertyIndex] = prop; propertiesValues[propertyIndex].definition_changed = true; - this.props.update(propertiesValues); - // move the popover once the DOM is updated - this.shouldUpdatePopoverPosition = true; + this.props.update(propertiesValues).then(() => { + // move the popover once the DOM is updated + this.shouldUpdatePopoverPosition = true; + }); } /** @@ -320,6 +321,11 @@ export class PropertiesField extends Component { popover, { position: "top", margin: 10 }, ); + + const arrow = popover.querySelector(".popover-arrow"); + if (arrow) { + arrow.classList.add("d-none"); + } } /** diff --git a/addons/web/static/src/views/fields/properties/property_definition.js b/addons/web/static/src/views/fields/properties/property_definition.js index 6b3fe1afb39..cff4bbf2932 100644 --- a/addons/web/static/src/views/fields/properties/property_definition.js +++ b/addons/web/static/src/views/fields/properties/property_definition.js @@ -280,6 +280,20 @@ export class PropertyDefinition extends Component { this.state.propertyDefinition = propertyDefinition; } + /** + * We activate / deactivate the property in the kanban view. + * + * @param {boolean} newValue + */ + onViewInKanbanChange(newValue) { + const propertyDefinition = { + ...this.state.propertyDefinition, + view_in_kanban: newValue, + }; + this.props.onChange(propertyDefinition); + this.state.propertyDefinition = propertyDefinition; + } + /* -------------------------------------------------------- * Private methods * -------------------------------------------------------- */ diff --git a/addons/web/static/src/views/fields/properties/property_definition.scss b/addons/web/static/src/views/fields/properties/property_definition.scss index 96e34dd3c1e..ea47d5d265f 100644 --- a/addons/web/static/src/views/fields/properties/property_definition.scss +++ b/addons/web/static/src/views/fields/properties/property_definition.scss @@ -44,6 +44,7 @@ td { padding-bottom: 15px; } + .o_field_property_definition_kanban, .o_field_property_definition_value { .form-check-input { margin-left: 3px; @@ -55,3 +56,20 @@ display: inline-block !important; } } + +.o_field_property_definition { + .o_input_dropdown { + .o_properties_external_button, + .o_dropdown_button { + display: none; + } + + &:hover, + &:focus-within { + .o_properties_external_button, + .o_dropdown_button { + display: block; + } + } + } +} diff --git a/addons/web/static/src/views/fields/properties/property_definition.xml b/addons/web/static/src/views/fields/properties/property_definition.xml index 35e4fa8f845..2551d8d5ac9 100644 --- a/addons/web/static/src/views/fields/properties/property_definition.xml +++ b/addons/web/static/src/views/fields/properties/property_definition.xml @@ -164,6 +164,18 @@ /> + + + View In Kanban + + + + + diff --git a/addons/web/static/src/views/fields/properties/property_tags.js b/addons/web/static/src/views/fields/properties/property_tags.js index 3fd455f3202..763b17a4938 100644 --- a/addons/web/static/src/views/fields/properties/property_tags.js +++ b/addons/web/static/src/views/fields/properties/property_tags.js @@ -16,6 +16,10 @@ PropertyTagsColorListPopover.components = { ColorList, }; +// property tags does not really need timeout because it does not make RPC calls +export class PropertyTagAutoComplete extends AutoComplete { }; +Object.assign(PropertyTagAutoComplete, { timeout: 0 }); + export class PropertyTags extends Component { setup() { this.notification = useService("notification"); @@ -26,6 +30,15 @@ export class PropertyTags extends Component { * Public methods / Getters * -------------------------------------------------------- */ + /** + * Return true if we should display the badges or just the tag label. + * + * @returns {array} + */ + get displayBadge() { + return !this.env.config || this.env.config.viewType !== "kanban"; + } + /** * Return the list containing tags values and actions for the TagsList component. * @@ -38,7 +51,12 @@ export class PropertyTags extends Component { // Retrieve the tags label and color // ['a', 'b'] => [['a', 'A', 5], ['b', 'B', 6]] - const value = this.props.tags.filter((tag) => this.props.selectedTags.indexOf(tag[0]) >= 0); + let value = this.props.tags.filter((tag) => this.props.selectedTags.indexOf(tag[0]) >= 0); + + if (!this.displayBadge) { + // in kanban view e.g. to not show tag without color + value = value.filter(tag => tag[2]); + } const canDeleteTag = !this.props.readonly && this.props.canChangeTags; @@ -262,7 +280,7 @@ export class PropertyTags extends Component { PropertyTags.template = "web.PropertyTags"; PropertyTags.components = { - AutoComplete, + AutoComplete: PropertyTagAutoComplete, TagsList, ColorList, Popover: PropertyTagsColorListPopover, diff --git a/addons/web/static/src/views/fields/properties/property_tags.scss b/addons/web/static/src/views/fields/properties/property_tags.scss index eb6f463a857..f9d64d257ae 100644 --- a/addons/web/static/src/views/fields/properties/property_tags.scss +++ b/addons/web/static/src/views/fields/properties/property_tags.scss @@ -5,8 +5,6 @@ } .o_field_property_tag { - border: 1px solid var(--o-input-border-color); - .o_tag { margin: 1px 2px 1px 0; } @@ -33,3 +31,8 @@ } } } + +.o_field_property_tag:not(.readonly):hover, +.o_field_property_tag:not(.readonly):focus-within { + border-bottom: 1px solid var(--o-input-border-color); +} diff --git a/addons/web/static/src/views/fields/properties/property_tags.xml b/addons/web/static/src/views/fields/properties/property_tags.xml index 828bf556eb8..7f90b9f48b5 100644 --- a/addons/web/static/src/views/fields/properties/property_tags.xml +++ b/addons/web/static/src/views/fields/properties/property_tags.xml @@ -2,9 +2,10 @@ -
+
await this._openRecord(this.props.comodel, many2manyValue[0])), onDelete: !this.props.readonly && hasAccess @@ -164,8 +164,19 @@ export class PropertyValue extends Component { return value.toString(); } + /** + * Return true if the relational properties are clickable. + * + * @returns {boolean} + */ + get clickableRelational() { + return !this.env.config || this.env.config.viewType !== "kanban"; + } + /** * Return True if we need to display a avatar for the current property. + * + * @returns {boolean} */ get showAvatar() { return ["many2one", "many2many"].includes(this.props.type) diff --git a/addons/web/static/src/views/fields/properties/property_value.scss b/addons/web/static/src/views/fields/properties/property_value.scss index a2af6c7f08f..d838f3f61fd 100644 --- a/addons/web/static/src/views/fields/properties/property_value.scss +++ b/addons/web/static/src/views/fields/properties/property_value.scss @@ -39,10 +39,9 @@ input.o_field_property_input[type=number] { } .o_field_property_many2many_value { - .o_tag, - .o_tag_badge_text, - .o_delete { - color: $gray-400 !important; + .o_tag { + color: nth($o-colors, 1) !important; + box-shadow: inset 0 0 0 1px !important; } .o_delete { line-height: 1; @@ -56,6 +55,33 @@ input.o_field_property_input[type=number] { } } -.o_form_readonly .o_field_property_many2many_value { - border: 0px; +.o_field_property_many2many_value:not(.readonly):hover, +.o_field_property_many2many_value:not(.readonly):focus-within { + border-bottom: 1px solid var(--o-input-border-color); +} + +.o_field_property_many2many_value.avatar .o_tag{ + img { + margin: -0.3em; + margin-right: 0.3em; + } + + .o_tag_badge_text { + padding: 0px !important; + } +} + +.o_property_field_value { + .o_properties_external_button, + .o_dropdown_button { + display: none; + } + + &:hover, + &:focus-within { + .o_properties_external_button, + .o_dropdown_button { + display: block; + } + } } diff --git a/addons/web/static/src/views/fields/properties/property_value.xml b/addons/web/static/src/views/fields/properties/property_value.xml index 917710c4443..ad4e2f4ffe1 100644 --- a/addons/web/static/src/views/fields/properties/property_value.xml +++ b/addons/web/static/src/views/fields/properties/property_value.xml @@ -70,14 +70,19 @@ - - - No Access - + + + + No Access + + +
-
+
{ string: "My Char", type: "char", value: "char value", + view_in_kanban: true, }, { name: "property_2", @@ -69,6 +70,7 @@ QUnit.module("Fields", (hooks) => { ], value: "b", default: "c", + view_in_kanban: true, }, ], company_id: 37, @@ -82,6 +84,7 @@ QUnit.module("Fields", (hooks) => { string: "My Char", type: "char", value: "char value", + view_in_kanban: true, }, { name: "property_2", @@ -94,6 +97,7 @@ QUnit.module("Fields", (hooks) => { ], value: "c", default: "c", + view_in_kanban: true, }, { name: "property_3", @@ -106,6 +110,7 @@ QUnit.module("Fields", (hooks) => { string: "My Char 4", type: "char", value: "char value 4", + view_in_kanban: true, }, ], company_id: 37, @@ -1052,4 +1057,42 @@ QUnit.module("Fields", (hooks) => { const propertyName6 = property.getAttribute("property-name"); assert.strictEqual(propertyName4, propertyName6); }); + + + /** + * Check the behavior of the properties field in the kanban view. + */ + QUnit.test("properties: kanban view", async function (assert) { + await makeView({ + type: "kanban", + resModel: "partner", + serverData, + arch: ` + + + +
+
+
+ +
+
+
+
`, + }); + + // check second card + const property3 = target.querySelector(".o_kanban_record:nth-child(2) .o_kanban_property_field:nth-child(3) span"); + assert.notEqual(property3.innerText, "char value 3", + "The third property should not be visible in the kanban view"); + assert.equal(property3.innerText, "char value 4"); + const property1 = target.querySelector(".o_kanban_record:nth-child(2) .o_kanban_property_field:nth-child(1) span"); + assert.equal(property1.innerText, "char value"); + const property2 = target.querySelector(".o_kanban_record:nth-child(2) .o_kanban_property_field:nth-child(2) span"); + assert.equal(property2.innerText, "C"); + + // check first card + const items = target.querySelectorAll(".o_kanban_record:nth-child(1) .o_kanban_property_field"); + assert.equal(items.length, 2); + }); }); diff --git a/odoo/addons/test_new_api/tests/test_properties.py b/odoo/addons/test_new_api/tests/test_properties.py index f2bb8df2534..913d59a3ae8 100644 --- a/odoo/addons/test_new_api/tests/test_properties.py +++ b/odoo/addons/test_new_api/tests/test_properties.py @@ -161,6 +161,7 @@ class PropertiesCase(TransactionCase): self.assertEqual(sql_values_1, {'discussion_color_code': 'orange', 'moderator_partner_id': self.partner_2.id, 'state': 'done'}) self.assertEqual(sql_values_3, {'discussion_color_code': 'orange', 'moderator_partner_id': self.partner_2.id, 'state': 'done'}) + @mute_logger('odoo.models.unlink', 'odoo.fields') def test_properties_field_read_batch(self): values = self.message_1.read(['attributes'])[0]['attributes'] self.assertEqual(len(values), 2) @@ -186,11 +187,35 @@ class PropertiesCase(TransactionCase): with self.assertQueryCount(5), self.assertQueries(expected_queries): self.message_1.read(['attributes']) + # read in batch a lot of records + discussions = [self.discussion_1, self.discussion_2] + partners = self.env['test_new_api.partner'].create([{'name': f'Test {i}'} for i in range(50)]) + messages = self.env['test_new_api.message'].create([{ + 'name': f'Test Message {i}', + 'discussion': discussions[i % 2].id, + 'author': self.user.id, + 'attributes': [{ + 'name': 'partner_id', + 'type': 'many2one', + 'comodel': 'test_new_api.partner', + 'value': partner.id, + 'definition_changed': True, + }] + } for i, partner in enumerate(partners)]) + self.env.invalidate_all() - expected_queries += expected_queries[-2:] - with self.assertQueryCount(7), self.assertQueries(expected_queries): - # 2 more queries for message 2 to verify his partner existence / name_get - (self.message_1 | self.message_2).read(['attributes']) + + with self.assertQueryCount(5), self.assertQueries(expected_queries): + values = messages.read(['attributes']) + + # remove some partners in the list + partners[:20].unlink() + self.env.invalidate_all() + # 5 queries instead of 25 queries, thanks to the cache values that has been + # cleaned (the properties field can trust the cached value, the deleted ids + # are not in the cache even if they still exists in the database) + with self.assertQueryCount(5): + values = messages.read(['attributes']) @mute_logger('odoo.fields') def test_properties_field_delete(self): @@ -824,7 +849,7 @@ class PropertiesCase(TransactionCase): 'comodel': 'test_new_api.partner', }] - with self.assertQueryCount(4): + with self.assertQueryCount(2): self.message_1.attributes = [ { "name": "moderator_partner_ids", @@ -837,7 +862,7 @@ class PropertiesCase(TransactionCase): self.assertEqual(self.message_1.attributes[0]['value'], partners[:10].ids) partners[:5].unlink() - with self.assertQueryCount(4): + with self.assertQueryCount(5): self.assertEqual(self.message_1.attributes[0]['value'], partners[5:10].ids) partners[5].unlink() @@ -847,7 +872,9 @@ class PropertiesCase(TransactionCase): # need to wait next write to clean data in database # a single read won't clean the removed many2many - self.message_1.attributes = self.message_1.read(['attributes'])[0]['attributes'] + attributes = self.message_1.read(['attributes'])[0]['attributes'] + self.message_1.invalidate_recordset() + self.message_1.attributes = attributes sql_values = self._get_sql_properties(self.message_1) self.assertEqual(sql_values, {'moderator_partner_ids': partners[6:10].ids}) @@ -915,7 +942,7 @@ class PropertiesCase(TransactionCase): }]) @users('test') - @mute_logger('odoo.addons.base.models.ir_rule') + @mute_logger('odoo.addons.base.models.ir_rule', 'odoo.fields') def test_properties_field_many2many_filtering(self): # a user read a properties with a many2many and he doesn't have access to all records tags = self.env['test_new_api.multi.tag'].create( @@ -949,14 +976,12 @@ class PropertiesCase(TransactionCase): self.assertEqual(values, [(tag.id, None if i >= 5 else tag.name) for i, tag in enumerate(tags.sudo())]) def test_properties_field_performance(self): - with self.assertQueryCount(4): - self.message_1.attributes + self.env.invalidate_all() + with self.assertQueryCount(5): + # read to put the partner name in cache + self.message_1.read(['attributes']) - expected = ['SELECT "test_new_api_partner".id FROM "test_new_api_partner" WHERE "test_new_api_partner".id IN %s'] - with self.assertQueryCount(1, msg='Must read value from cache'), self.assertQueries(expected): - # still cost 1 SQL query to check existence because the ORM stores - # the raw SQL response in cache (not the result of convert_to_cache) - # so the value in cache is not verified (see models.py@_read) + with self.assertQueryCount(0, msg='Must read value from cache'): self.message_1.attributes expected = ['UPDATE "test_new_api_message" SET "attributes" = %s, "write_date" = %s, "write_uid" = %s WHERE id IN %s'] @@ -1086,6 +1111,7 @@ class PropertiesCase(TransactionCase): # change the definition record, change the definition and add default values self.assertEqual(message.discussion, self.discussion_2) + with self.assertQueryCount(4): message.discussion = self.discussion_1 self.assertEqual( @@ -1265,8 +1291,10 @@ class PropertiesCase(TransactionCase): """Check the access right related to the Properties fields.""" MultiTag = type(self.env['test_new_api.multi.tag']) - def _mocked_check_access_rights(*args, **kwargs): - raise AccessError('') + def _mocked_check_access_rights(operation, raise_exception=True): + if raise_exception: + raise AccessError('') + return False # a user read a properties with a many2one to a record he doesn't have access to tag = self.env['test_new_api.multi.tag'].create({'name': 'Test Tag'}) diff --git a/odoo/fields.py b/odoo/fields.py index f6a88aefc4a..c502ed52e3b 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -3257,7 +3257,7 @@ class Properties(Field): if not value or not definition: return definition or [] - assert isinstance(value, dict) + assert isinstance(value, dict), f"Wrong type {value!r}" value = self._dict_to_list(value, definition) self._parse_json_types(value, record.env) @@ -3299,6 +3299,79 @@ class Properties(Field): self._add_display_name(value, record.env) return value + def read(self, records): + """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 + convert_to_record / convert_to_read. + """ + definition_records_map = { + record: record[self.definition_record][self.definition_record_field] + for record in records + } + + # 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: + 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 + + default = property_definition.get('default') or [] + property_value = record_values[name] or [] + + if type_ == 'many2one': + default = [default] if default else [] + property_value = [property_value] if property_value else [] + + ids_per_model[comodel].update(default) + ids_per_model[comodel].update(property_value) + + # check existence and pre-fetch in batch + existing_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 + + # 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 + + 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) + def write(self, records, value): """Check if the properties definition has been changed. @@ -3487,7 +3560,7 @@ class Properties(Field): definition['name'] = str(uuid.uuid4()).replace('-', '')[:16] @classmethod - def _parse_json_types(cls, values_list, env, check_existence=True): + def _parse_json_types(cls, values_list, env): """Parse the value stored in the JSON. Check for records existence, if we removed a selection option, ... @@ -3529,10 +3602,6 @@ class Properties(Field): if not isinstance(property_value, int): raise ValueError(f'Wrong many2one value: {property_value!r}.') - if check_existence and property_value: - # many2one might have been deleted - property_value = env[res_model].browse(property_value).exists().id - 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}.') @@ -3541,10 +3610,6 @@ class Properties(Field): # remove duplicated value and preserve order property_value = list(dict.fromkeys(property_value)) - elif check_existence and property_value: - # many2many might have been deleted - property_value = env[res_model].browse(property_value).exists().ids - property_definition['value'] = property_value @classmethod @@ -3634,7 +3699,7 @@ class PropertiesDefinition(Field): REQUIRED_KEYS = ('name', 'type') ALLOWED_KEYS = ( 'name', 'string', 'type', 'comodel', 'default', - 'selection', 'tags', 'domain', + 'selection', 'tags', 'domain', 'view_in_kanban', ) def convert_to_column(self, value, record, values=None, validate=True): diff --git a/odoo/models.py b/odoo/models.py index 180dc0dbb62..e684da6f886 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3185,6 +3185,12 @@ class BaseModel(metaclass=MetaModel): if field.store and field.translate: translated_field_names.append(field.name) + if field.type == 'properties': + # force calling fields.read for properties field because + # we want to read all relational properties in batch + # (and check their existence in batch as well) + other_fields.append(field) + if column_fields: cr, context = self.env.cr, self.env.context