From fa67f44794cb254860f7df8e1149186e1a650381 Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Wed, 21 Sep 2022 13:40:56 +0000 Subject: [PATCH] [IMP] base, web: do not pass field readonly/required for readonly views pivot, graph, cohort views do not care to know whether a field is readonly or required, as you cannot edit records in these views. Even kanban is readonly in most cases: - you can drag and drop records from one column to another, which is prevented if the group by field is readonly but this shouldn't rely on the fact the field is within the architecture, as you can group by on any fields from the search views / control panel. Hence, this shouldn't rely entirely on the modifiers passed on the field nodes in the view architecture alone. - you can create new record inside the kanban, with a simplified form, thanks to the `quick_create`, but this uses an independant form view, in which the readonly and required modifiers are correctly passed. So, `modifiers="{'readonly': true, 'required': true}"` can be dropped for kanban views as well. This allow to spare some KB by not setting useless modifiers in views. e.g. CRM > My pipeline pivot Before ```xml ``` After ```xml ``` Regarding the change of behavior shown in `addons/web/static/tests/views/kanban_view_tests.js`. It was introduced very recently, by myself, in odoo/odoo#100806 I revert this possibility to set readonly="0" on a field node in a kanban view, because: - First, this is not used anywhere in both odoo/odoo and odoo/enterprise. - Second, this really makes things harder if we want to do so: - as readonly="0" is passed, the "readonly" gets removed from the node modifiers, as they are simplified by removing falsy value: modifiers="{'invisible: True, 'readonly': False}" becomes modifiers="{'invisible': True}" - as readonly in not amongst the modifiers, it fallbacks on the model field property, in the javascript code, which is readonly: True. - the thing to do would be to still transfer "readonly" from the field attributes to the node modifiers. - which either mean to consider a kanban view as editable - this will cause issues because the validation mechanism will suddenly check the domain attribute property https://github.com/odoo/odoo/blob/d4a92b112d0554a2624f7768feb7d54e0484469f/odoo/addons/base/models/ir_ui_view.py#L1443 and there will be plenty of views where some field used in the domains will be missing. Besides it is pointless to validate these domains as they are completely unused in kanban views - either mean to find another mechanism than "editable" to decide wheter to transfer the modifiers "readonly"/"required" or not, which over-complicates things. - besides only "readonly" would need to be passed, not "required. So, to keep the code stupid simple, I remove this possibility added only a few days ago, which is actually not used anywhere in standard for the moment. closes odoo/odoo#102221 X-original-commit: 69c3d5aa25655173ed66a52570559322c650c7df Signed-off-by: Denis Ledoux (dle) --- .../src/views/kanban/kanban_renderer.js | 4 +-- .../web/static/tests/helpers/mock_server.js | 2 +- .../fields/boolean_toggle_field_tests.js | 4 +-- .../static/tests/views/kanban_view_tests.js | 26 +++++++------------ odoo/addons/base/models/ir_ui_view.py | 10 ++++--- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/addons/web/static/src/views/kanban/kanban_renderer.js b/addons/web/static/src/views/kanban/kanban_renderer.js index 60bb00b7b13..54ec58052dc 100644 --- a/addons/web/static/src/views/kanban/kanban_renderer.js +++ b/addons/web/static/src/views/kanban/kanban_renderer.js @@ -161,7 +161,7 @@ export class KanbanRenderer extends Component { const { groupByField, fields } = this.props.list; const { modifiers, type } = groupByField; return Boolean( - !(modifiers ? modifiers.readonly : fields[groupByField.name].readonly) && + !(modifiers && "readonly" in modifiers ? modifiers.readonly : fields[groupByField.name].readonly) && (isAllowedDateField(groupByField) || MOVABLE_RECORD_TYPES.includes(type)) ); } @@ -175,7 +175,7 @@ export class KanbanRenderer extends Component { const { groupsDraggable } = this.props.archInfo; return ( groupsDraggable && - !(modifiers ? modifiers.readonly : fields[groupByField.name].readonly) && + !(modifiers && "readonly" in modifiers ? modifiers.readonly : fields[groupByField.name].readonly) && DRAGGABLE_GROUP_TYPES.includes(type) ); } diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index d65533fda78..25d20794f33 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -285,7 +285,7 @@ export class MockServer { } const defaultValues = {}; const stateExceptions = {}; // what is this ? - modifiersNames.forEach((attr) => { + (editableView && modifiersNames || ["invisible"]).forEach((attr) => { stateExceptions[attr] = []; defaultValues[attr] = !!field[attr]; }); diff --git a/addons/web/static/tests/views/fields/boolean_toggle_field_tests.js b/addons/web/static/tests/views/fields/boolean_toggle_field_tests.js index 558154e33b4..b61ae2927dc 100644 --- a/addons/web/static/tests/views/fields/boolean_toggle_field_tests.js +++ b/addons/web/static/tests/views/fields/boolean_toggle_field_tests.js @@ -204,9 +204,9 @@ QUnit.module("Fields", (hooks) => { serverData, resModel: "partner", arch: ` - + - + `, }); diff --git a/addons/web/static/tests/views/kanban_view_tests.js b/addons/web/static/tests/views/kanban_view_tests.js index 5b001c0000c..37a281a961f 100644 --- a/addons/web/static/tests/views/kanban_view_tests.js +++ b/addons/web/static/tests/views/kanban_view_tests.js @@ -4333,19 +4333,18 @@ QUnit.module("Views", (hooks) => { }); QUnit.test("prevent drag and drop of record if grouped by readonly", async (assert) => { - // Whether the kanban is grouped by state, foo or bar + // Whether the kanban is grouped by state, foo, bar or product_id // the user must not be able to drag and drop from one group to another, - // as state, foo or bar are made readonly one way or another. - // However, product_id must be draggable: by default, in the models, it's readonly, - // but a counter order is given in the view architecture: readonly="0". + // as state, foo bar, product_id are made readonly one way or another. // state must not be draggable: // state is not readonly in the model. state is passed in the arch specifying readonly="1". // foo must not be draggable: // foo is readonly in the model fields. foo is passed in the arch but without specifying readonly. // bar must not be draggable: // bar is readonly in the model fields. bar is not passed in the arch. - // product_id must be draggable: - // product_id is readonly in the model fields. product_id is passed in the arch specifying readonly="0". + // product_id must not be draggable: + // product_id is readonly in the model fields. product_id is passed in the arch specifying readonly="0", + // but the readonly in the model takes over. serverData.models.partner.fields.foo.readonly = true; serverData.models.partner.fields.bar.readonly = true; serverData.models.partner.fields.product_id.readonly = true; @@ -4368,12 +4367,7 @@ QUnit.module("Views", (hooks) => { if (route === "/web/dataset/resequence") { return true; } - if ( - args.model === "partner" && - args.method === "write" && - !(args.args && args.args[1] && args.args[1].product_id) - ) { - // In the test, nothing should be draggable except the test on product_id + if (args.model === "partner" && args.method === "write") { throw new Error("should not be draggable"); } }, @@ -4457,12 +4451,12 @@ QUnit.module("Views", (hooks) => { ".o_kanban_group:nth-child(2)" ); - // should be draggable - assert.containsN(target, ".o_kanban_group:first-child .o_kanban_record", 1); - assert.containsN(target, ".o_kanban_group:nth-child(2) .o_kanban_record", 3); + // should not be draggable + assert.containsN(target, ".o_kanban_group:first-child .o_kanban_record", 2); + assert.containsN(target, ".o_kanban_group:nth-child(2) .o_kanban_record", 2); assert.containsN(target, ".o_kanban_group:nth-child(3) .o_kanban_record", 0); - assert.deepEqual(getCardTexts(0), ["gnapGHI"]); + assert.deepEqual(getCardTexts(0), ["yopABC", "gnapGHI"]); }); QUnit.test("prevent drag and drop if grouped by date/datetime field", async (assert) => { diff --git a/odoo/addons/base/models/ir_ui_view.py b/odoo/addons/base/models/ir_ui_view.py index 15afebe27bf..53bf22c6fd7 100644 --- a/odoo/addons/base/models/ir_ui_view.py +++ b/odoo/addons/base/models/ir_ui_view.py @@ -59,15 +59,16 @@ def att_names(name): yield f"t-attf-{name}" -def transfer_field_to_modifiers(field, modifiers): +def transfer_field_to_modifiers(field, modifiers, view_editable=True): default_values = {} state_exceptions = {} - for attr in ('invisible', 'readonly', 'required'): + attributes = ('invisible', 'readonly', 'required') if view_editable else ('invisible',) + for attr in attributes: state_exceptions[attr] = [] default_values[attr] = bool(field.get(attr)) for state, modifs in field.get("states", {}).items(): for modif in modifs: - if default_values[modif[0]] != modif[1]: + if modif[0] in attributes and default_values[modif[0]] != modif[1]: state_exceptions[modif[0]].append(state) for attr, default_value in default_values.items(): @@ -1123,6 +1124,7 @@ actual arch. root_info = { 'view_type': root.tag, + 'view_editable': editable and self._editable_node(root, name_manager), 'mobile': options.get('mobile'), } @@ -1258,7 +1260,7 @@ actual arch. field_info = name_manager.field_info.get(node.get('name')) if field_info: - transfer_field_to_modifiers(field_info, node_info['modifiers']) + transfer_field_to_modifiers(field_info, node_info['modifiers'], node_info['view_editable']) def _postprocess_tag_form(self, node, name_manager, node_info): result = name_manager.model.view_header_get(False, node.tag)