From 0501bbd62e517f6c215d9e7e36d61747c7f5816b Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Wed, 18 May 2022 13:30:12 +0000 Subject: [PATCH] [IMP] base: uniform "groups" in back-end view This revision is to make uniform the behavior of the `groups` attribute on the Python model fields and on the node in the view architecture. In both cases, remove the node from the view completely. Before this revision, in a back-end view: - In the Python model, if a field has the `groups` attribute set and the user is not part of the groups, the field is removed, completely, from the view. - In the view architecture, if a node has the `groups` attribute set and the user is not part of the groups, the node is made invisible (not completely removed, just made invisible). in a front-end view: - if a node has a "groups" or "t-groups" set and the user is not part of the groups, the node is removed from the view. So it's 2/3 cases removing nodes restricted to a group. and 1/3 case making invisible nodes restricted to a group. It's simpler to have a uniform behavior for the 3 cases, simpler to understandard for developers. In addition, this will help for the goal to cache back-end views. It makes possible to convert views using the `groups_id` field by moving the content of these views directly in the view to which they add content which is suppose to be completely removed when the user has not the according group. By getting rid of the `groups_id` many2many field on `ir.ui.view`, it makes possible to cache the view architecture without requiring to use the groups in the cache key. Currently, if we want to cache the view architecture, it would be required to use the intersection of the user groups with the `groups_id` groups of the view, making it costly to compute the cache key, therefore altering the performance point to cache the view architectures. closes odoo/odoo#95729 Related: odoo/enterprise#29592 Signed-off-by: Denis Ledoux (dle) --- addons/account/models/account_move.py | 6 +- .../account/views/account_account_views.xml | 1 + .../account/views/account_journal_views.xml | 5 + addons/account/views/account_move_views.xml | 7 + addons/account/views/account_tax_views.xml | 5 +- addons/account/views/partner_view.xml | 1 + .../analytic/views/analytic_account_views.xml | 3 + addons/crm/tests/test_res_partner.py | 8 +- addons/crm/views/crm_lead_views.xml | 1 + addons/crm/views/crm_team_views.xml | 1 + addons/crm/views/res_partner_views.xml | 3 + .../crm_iap_lead_mining_request_views.xml | 1 + addons/delivery/views/delivery_view.xml | 1 + addons/event/views/event_event_views.xml | 1 + .../event_crm/views/event_lead_rule_views.xml | 1 + .../fleet/views/fleet_vehicle_cost_views.xml | 1 + addons/fleet/views/fleet_vehicle_views.xml | 1 + addons/hr/views/hr_department_views.xml | 1 + addons/hr/views/hr_employee_views.xml | 2 + addons/hr/views/hr_job_views.xml | 1 + addons/hr/views/hr_plan_views.xml | 2 + addons/hr/views/hr_work_location_views.xml | 1 + .../hr_contract/views/hr_contract_views.xml | 3 + addons/hr_expense/views/hr_expense_views.xml | 3 + .../views/hr_leave_allocation_views.xml | 4 +- .../views/hr_leave_stress_day_views.xml | 1 + addons/hr_holidays/views/hr_leave_views.xml | 1 - addons/hr_holidays/views/hr_views.xml | 1 + addons/hr_holidays/views/resource_views.xml | 1 + .../views/hr_recruitment_views.xml | 1 + .../hr_timesheet/views/hr_timesheet_views.xml | 1 + addons/hr_timesheet/views/project_views.xml | 2 + .../loyalty/views/loyalty_program_views.xml | 1 + addons/lunch/views/lunch_product_views.xml | 1 + addons/mail/views/fetchmail_views.xml | 2 +- addons/mail/views/ir_model_views.xml | 2 + addons/mail/views/mail_activity_views.xml | 1 + addons/mail/views/mail_mail_views.xml | 2 + .../maintenance/views/maintenance_views.xml | 3 + .../views/mailing_mailing_views.xml | 2 + .../mass_mailing/views/utm_campaign_views.xml | 2 +- .../mass_mailing_event/views/event_views.xml | 1 + .../views/event_views.xml | 1 + .../views/slide_channel_views.xml | 1 + .../views/utm_campaign_views.xml | 4 +- addons/mrp/tests/test_order.py | 4 + addons/mrp/views/mrp_bom_views.xml | 8 +- addons/mrp/views/mrp_production_views.xml | 27 + addons/mrp/views/mrp_routing_views.xml | 1 + addons/mrp/views/mrp_unbuild_views.xml | 2 + addons/mrp/views/mrp_workcenter_views.xml | 4 +- addons/mrp/views/mrp_workorder_views.xml | 1 + .../views/stock_landed_cost_views.xml | 1 + .../views/stock_move_views.xml | 3 +- .../payment/views/payment_acquirer_views.xml | 2 + .../product/views/product_packaging_views.xml | 1 + .../views/product_pricelist_item_views.xml | 2 + addons/product/views/product_views.xml | 3 +- addons/project/static/src/js/project_form.js | 4 +- addons/project/views/project_views.xml | 7 + .../views/hr_holidays_views.xml | 1 + addons/purchase/views/product_views.xml | 3 + addons/purchase/views/purchase_views.xml | 2 + .../views/purchase_requisition_views.xml | 1 + addons/repair/views/repair_views.xml | 1 + addons/resource/views/resource_views.xml | 3 + addons/sale/views/sale_order_views.xml | 9 +- addons/sale_crm/views/partner_views.xml | 6 + .../views/sale_order_template_views.xml | 1 + addons/sale_margin/views/sale_order_views.xml | 1 + .../sale_project/views/sale_order_views.xml | 4 +- .../views/project_task_views.xml | 1 + .../views/crm_team_member_views.xml | 1 + addons/stock/views/product_strategy_views.xml | 2 + addons/stock/views/product_views.xml | 2 + addons/stock/views/stock_location_views.xml | 2 + addons/stock/views/stock_lot_views.xml | 1 + addons/stock/views/stock_move_line_views.xml | 3 + addons/stock/views/stock_move_views.xml | 8 + addons/stock/views/stock_orderpoint_views.xml | 2 + .../stock/views/stock_package_level_views.xml | 1 + addons/stock/views/stock_picking_views.xml | 4 + addons/stock/views/stock_quant_views.xml | 3 + addons/stock/views/stock_rule_views.xml | 4 + addons/stock/views/stock_scrap_views.xml | 1 + .../views/stock_storage_category_views.xml | 2 + addons/stock/views/stock_warehouse_views.xml | 1 + .../stock/wizard/stock_inventory_conflict.xml | 1 + addons/stock/wizard/stock_picking_return.py | 11 +- addons/stock_account/views/product_views.xml | 1 + .../views/stock_landed_cost_views.xml | 1 + .../views/stock_picking_batch_views.xml | 2 + addons/survey/views/survey_user_views.xml | 1 + .../views/crm_reveal_views.xml | 1 + .../website_event/views/event_event_views.xml | 1 + addons/website_sale/views/product_views.xml | 1 + .../wizard/sale_coupon_share_views.xml | 1 + .../views/res_config_settings_views.xml | 1 + odoo/addons/base/models/ir_ui_view.py | 186 +++++-- odoo/addons/base/models/res_users.py | 8 +- odoo/addons/base/tests/test_acl.py | 11 +- odoo/addons/base/tests/test_views.py | 475 ++++++++++++++++++ odoo/addons/base/views/ir_model_views.xml | 1 + odoo/addons/base/views/ir_module_views.xml | 5 +- odoo/addons/base/views/res_partner_views.xml | 1 + 105 files changed, 875 insertions(+), 69 deletions(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 5d6f70d3277..e6c13c71d68 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -2175,7 +2175,11 @@ class AccountMove(models.Model): for key in list(field_onchange): if key == to_del or key.startswith(f"{to_del}."): del field_onchange[key] - del values[to_del] + # test_01_account_tour + # File "/data/build/odoo/addons/account/models/account_move.py", line 2127, in onchange + # del values[to_del] + # KeyError: 'line_ids' + values.pop(to_del, None) return super().onchange(values, field_name, field_onchange) # ------------------------------------------------------------------------- diff --git a/addons/account/views/account_account_views.xml b/addons/account/views/account_account_views.xml index 5b61204f3ed..c6966016367 100644 --- a/addons/account/views/account_account_views.xml +++ b/addons/account/views/account_account_views.xml @@ -89,6 +89,7 @@ account.account + diff --git a/addons/account/views/account_journal_views.xml b/addons/account/views/account_journal_views.xml index ff4647fda9c..fbe4888d8e3 100644 --- a/addons/account/views/account_journal_views.xml +++ b/addons/account/views/account_journal_views.xml @@ -27,6 +27,9 @@ 1
+ + +
+ + + {'default_is_company': True, 'show_vat': True, 'default_user_id': user_id, 'default_team_id': team_id} diff --git a/addons/crm_iap_mine/views/crm_iap_lead_mining_request_views.xml b/addons/crm_iap_mine/views/crm_iap_lead_mining_request_views.xml index c2f2fba67a6..09a001235aa 100644 --- a/addons/crm_iap_mine/views/crm_iap_lead_mining_request_views.xml +++ b/addons/crm_iap_mine/views/crm_iap_lead_mining_request_views.xml @@ -12,6 +12,7 @@ + diff --git a/addons/event_crm/views/event_lead_rule_views.xml b/addons/event_crm/views/event_lead_rule_views.xml index 832a0501f42..61ebff9d44f 100644 --- a/addons/event_crm/views/event_lead_rule_views.xml +++ b/addons/event_crm/views/event_lead_rule_views.xml @@ -42,6 +42,7 @@

+ diff --git a/addons/fleet/views/fleet_vehicle_cost_views.xml b/addons/fleet/views/fleet_vehicle_cost_views.xml index 4351a5b44db..d4d0a76f913 100644 --- a/addons/fleet/views/fleet_vehicle_cost_views.xml +++ b/addons/fleet/views/fleet_vehicle_cost_views.xml @@ -5,6 +5,7 @@ fleet.vehicle.log.contract +
+
diff --git a/addons/hr/views/hr_department_views.xml b/addons/hr/views/hr_department_views.xml index cb6e201cf21..907ac938544 100644 --- a/addons/hr/views/hr_department_views.xml +++ b/addons/hr/views/hr_department_views.xml @@ -6,6 +6,7 @@ hr.department +
- diff --git a/addons/hr_timesheet/views/project_views.xml b/addons/hr_timesheet/views/project_views.xml index 1a5ed320eed..60ebf27bc8d 100644 --- a/addons/hr_timesheet/views/project_views.xml +++ b/addons/hr_timesheet/views/project_views.xml @@ -243,6 +243,7 @@ + @@ -272,6 +273,7 @@ + diff --git a/addons/loyalty/views/loyalty_program_views.xml b/addons/loyalty/views/loyalty_program_views.xml index bc20aafeed1..593ae61a4cb 100644 --- a/addons/loyalty/views/loyalty_program_views.xml +++ b/addons/loyalty/views/loyalty_program_views.xml @@ -29,6 +29,7 @@
+
+ + + diff --git a/addons/sale_crm/views/partner_views.xml b/addons/sale_crm/views/partner_views.xml index a43205619dd..221f2cf56af 100644 --- a/addons/sale_crm/views/partner_views.xml +++ b/addons/sale_crm/views/partner_views.xml @@ -12,6 +12,12 @@ account.group_delivery_invoice_address + + + + + + diff --git a/addons/sale_management/views/sale_order_template_views.xml b/addons/sale_management/views/sale_order_template_views.xml index e3cc5de44a6..ef4af697e92 100644 --- a/addons/sale_management/views/sale_order_template_views.xml +++ b/addons/sale_management/views/sale_order_template_views.xml @@ -32,6 +32,7 @@ + + diff --git a/addons/stock/views/stock_storage_category_views.xml b/addons/stock/views/stock_storage_category_views.xml index bdf97bc5da6..af73e63ca6f 100644 --- a/addons/stock/views/stock_storage_category_views.xml +++ b/addons/stock/views/stock_storage_category_views.xml @@ -5,6 +5,7 @@ stock.storage.category +
+ + diff --git a/addons/website_event/views/event_event_views.xml b/addons/website_event/views/event_event_views.xml index 7d9b1382bf6..5deec13a959 100644 --- a/addons/website_event/views/event_event_views.xml +++ b/addons/website_event/views/event_event_views.xml @@ -37,6 +37,7 @@ + diff --git a/addons/website_sale/views/product_views.xml b/addons/website_sale/views/product_views.xml index 4d608494b2c..54774fcf122 100644 --- a/addons/website_sale/views/product_views.xml +++ b/addons/website_sale/views/product_views.xml @@ -283,6 +283,7 @@ + diff --git a/addons/website_sale_loyalty/wizard/sale_coupon_share_views.xml b/addons/website_sale_loyalty/wizard/sale_coupon_share_views.xml index e75866e90ee..46d8f202342 100644 --- a/addons/website_sale_loyalty/wizard/sale_coupon_share_views.xml +++ b/addons/website_sale_loyalty/wizard/sale_coupon_share_views.xml @@ -5,6 +5,7 @@ coupon.share +
diff --git a/addons/website_sale_stock/views/res_config_settings_views.xml b/addons/website_sale_stock/views/res_config_settings_views.xml index 6ee2626e340..b6c4b629e21 100644 --- a/addons/website_sale_stock/views/res_config_settings_views.xml +++ b/addons/website_sale_stock/views/res_config_settings_views.xml @@ -21,6 +21,7 @@
+
diff --git a/odoo/addons/base/models/ir_ui_view.py b/odoo/addons/base/models/ir_ui_view.py index d7d7864dad8..5ac18e17e74 100644 --- a/odoo/addons/base/models/ir_ui_view.py +++ b/odoo/addons/base/models/ir_ui_view.py @@ -974,16 +974,11 @@ actual arch. def _apply_groups(self, node, name_manager, node_info): """ Apply group restrictions: elements with a 'groups' attribute should - be made invisible to people who are not members. + be removed from the view to people who are not members. """ if node.get('groups'): - can_see = self.user_has_groups(groups=node.get('groups')) - if not can_see: - node.set('invisible', '1') - node_info['modifiers']['invisible'] = True - if 'attrs' in node.attrib: - del node.attrib['attrs'] # avoid making field visible later - del node.attrib['groups'] + if not self.user_has_groups(groups=node.get('groups')): + node.getparent().remove(node) def _get_view_refs(self, node): """ Extract the `[view_type]_view_ref` keys and values from the node context attribute, @@ -1145,10 +1140,10 @@ actual arch. def _postprocess_tag_calendar(self, node, name_manager, node_info): for additional_field in ('date_start', 'date_delay', 'date_stop', 'color', 'all_day'): if node.get(additional_field): - name_manager.has_field(node.get(additional_field).split('.', 1)[0]) + name_manager.has_field(node, node.get(additional_field).split('.', 1)[0]) for f in node: if f.tag == 'filter': - name_manager.has_field(f.get('name')) + name_manager.has_field(node, f.get('name')) def _postprocess_tag_field(self, node, name_manager, node_info): if node.get('name'): @@ -1203,7 +1198,7 @@ actual arch. node.set('can_create', 'true' if can_create else 'false') node.set('can_write', 'true' if can_write else 'false') - name_manager.has_field(node.get('name'), attrs) + name_manager.has_field(node, node.get('name'), attrs) field_info = name_manager.field_info.get(node.get('name')) if field_info: @@ -1225,7 +1220,7 @@ actual arch. self.with_context( base_model_name=name_manager.model._name, )._postprocess_view(node, field.comodel_name, editable=False, parent_name_manager=name_manager) - name_manager.has_field(name) + name_manager.has_field(node, name) def _postprocess_tag_label(self, node, name_manager, node_info): if node.get('for'): @@ -1355,10 +1350,10 @@ actual arch. def _validate_tag_calendar(self, node, name_manager, node_info): for additional_field in ('date_start', 'date_delay', 'date_stop', 'color', 'all_day'): if node.get(additional_field): - name_manager.has_field(node.get(additional_field).split('.', 1)[0]) + name_manager.has_field(node, node.get(additional_field).split('.', 1)[0]) for f in node: if f.tag == 'filter': - name_manager.has_field(f.get('name')) + name_manager.has_field(node, f.get('name')) def _validate_tag_search(self, node, name_manager, node_info): if node_info['validate'] and not node.iterdescendants(tag="field"): @@ -1397,7 +1392,7 @@ actual arch. fnames, vnames = self._get_domain_identifiers(node, domain, desc) self._check_field_paths(node, fnames, field.comodel_name, f"{desc} ({domain})") if vnames: - name_manager.must_have_fields(vnames, f"{desc} ({domain})") + name_manager.must_have_fields(node, vnames, f"{desc} ({domain})") elif validate and node.get('domain'): msg = _( @@ -1413,8 +1408,9 @@ actual arch. sub_manager = self._validate_view( child, field.comodel_name, editable=node_info['editable'], full=validate, ) - for fname, use in sub_manager.mandatory_parent_fields.items(): - name_manager.must_have_field(fname, use) + for fname, groups_uses in sub_manager.mandatory_parent_fields.items(): + for groups, use in groups_uses.items(): + name_manager.must_have_field(node, fname, use, groups=groups) elif validate and name not in name_manager.field_info: msg = _( @@ -1423,7 +1419,7 @@ actual arch. ) self._raise_view_error(msg, node) - name_manager.has_field(name, {'id': node.get('id'), 'select': node.get('select')}) + name_manager.has_field(node, name, {'id': node.get('id'), 'select': node.get('select')}) if validate: for attribute in ('invisible', 'readonly', 'required'): @@ -1447,7 +1443,7 @@ actual arch. fnames, vnames = self._get_domain_identifiers(node, domain, desc) self._check_field_paths(node, fnames, name_manager.model._name, f"{desc} ({domain})") if vnames: - name_manager.must_have_fields(vnames, f"{desc} ({domain})") + name_manager.must_have_fields(node, vnames, f"{desc} ({domain})") def _validate_tag_button(self, node, name_manager, node_info): if not node_info['validate']: @@ -1535,7 +1531,7 @@ actual arch. fnames, vnames = self._get_domain_identifiers(node, domain, desc) self._check_field_paths(node, fnames, field.comodel_name, f"{desc} ({domain})") if vnames: - name_manager.must_have_fields(vnames, f"{desc} ({domain})") + name_manager.must_have_fields(node, vnames, f"{desc} ({domain})") # move all children nodes into a new node groupby_node = E.groupby(*node) @@ -1543,9 +1539,10 @@ actual arch. sub_manager = self._validate_view( groupby_node, field.comodel_name, editable=False, full=node_info['validate'], ) - name_manager.has_field(name) - for fname, use in sub_manager.mandatory_parent_fields.items(): - name_manager.must_have_field(fname, use) + name_manager.has_field(node, name) + for fname, groups_uses in sub_manager.mandatory_parent_fields.items(): + for groups, use in groups_uses.items(): + name_manager.must_have_field(node, fname, use, groups=groups) elif node_info['validate']: msg = _( @@ -1639,11 +1636,11 @@ actual arch. # domains in attrs are used for readonly, invisible, ... # and thus are only executed client side fnames, vnames = self._get_domain_identifiers(node, val_ast, attr, expr) - name_manager.must_have_fields(fnames | vnames, f"attrs ({expr})") + name_manager.must_have_fields(node, fnames | vnames, f"attrs ({expr})") else: vnames = get_variable_names(val_ast) if vnames: - name_manager.must_have_fields(vnames, f"attrs ({expr})") + name_manager.must_have_fields(node, vnames, f"attrs ({expr})") elif attr == 'context': for key, val_ast in get_dict_asts(expr).items(): @@ -1665,7 +1662,7 @@ actual arch. else: vnames = get_variable_names(val_ast) if vnames: - name_manager.must_have_fields(vnames, f"context ({expr})") + name_manager.must_have_fields(node, vnames, f"context ({expr})") elif attr == 'groups': for group in expr.replace('!', '').split(','): @@ -1688,7 +1685,7 @@ actual arch. elif attr.startswith('decoration-'): vnames = get_variable_names(expr) if vnames: - name_manager.must_have_fields(vnames, f"{attr}={expr}") + name_manager.must_have_fields(node, vnames, f"{attr}={expr}") elif attr == 'data-bs-toggle' and expr == 'tab': if node.get('role') != 'tab': @@ -2194,11 +2191,11 @@ class NameManager: def __init__(self, model, parent=None): self.model = model - self.available_fields = collections.defaultdict(dict) # {field_name: field_info} + self.available_fields = collections.defaultdict(dict) # {field_name: {'groups': groups, 'info': field_info}} self.available_actions = set() self.available_names = set() - self.mandatory_fields = dict() # {field_name: use} - self.mandatory_parent_fields = dict() # {field_name: use} + self.mandatory_fields = collections.defaultdict(dict) # {field_name: {'groups': 'use}} + self.mandatory_parent_fields = collections.defaultdict(dict) # {field_name: {'groups': use}} self.mandatory_names = dict() # {name: use} self.parent = parent self.children = [] @@ -2215,26 +2212,36 @@ class NameManager: info['states'] = {} return field_info - def has_field(self, name, info=frozendict()): - self.available_fields[name].update(info) + def has_field(self, node, name, info=frozendict()): + groups = self._get_node_groups(node) + self.available_fields[name].setdefault('info', {}).update(info) + self.available_fields[name].setdefault('groups', []).append(groups) self.available_names.add(info.get('id') or name) def has_action(self, name): self.available_actions.add(name) - def must_have_field(self, name, use): - if name.startswith('parent.'): - self.mandatory_parent_fields[name[7:]] = use + def must_have_field(self, node, name, use, groups=None): + node_groups = self._get_node_groups(node) + if groups: + groups = groups + node_groups else: - self.mandatory_fields[name] = use + groups = node_groups + if name.startswith('parent.'): + self.mandatory_parent_fields[name[7:]][groups] = use + else: + self.mandatory_fields[name][groups] = use - def must_have_fields(self, names, use): + def must_have_fields(self, node, names, use, groups=None): for name in names: - self.must_have_field(name, use) + self.must_have_field(node, name, use, groups=groups) def must_have_name(self, name, use): self.mandatory_names[name] = use + def _get_node_groups(self, node): + return tuple(tuple(n.get('groups').split(',')) for n in node.xpath('ancestor-or-self::*[@groups]')) + def check(self, view): # context for translations below context = view.env.context # pylint: disable=unused-variable @@ -2252,7 +2259,8 @@ class NameManager: message = _("Field `%(name)s` does not exist", name=name) view._raise_view_error(message) - for name, use in self.mandatory_fields.items(): + for name, groups_uses in self.mandatory_fields.items(): + use = next(iter(groups_uses.values())) if name == 'id': # always available continue if "." in name: @@ -2261,7 +2269,7 @@ class NameManager: definition=name, use=use, ) view._raise_view_error(msg) - info = self.available_fields.get(name) + info = self.available_fields[name].get('info') if info is None: msg = _( "Field %(name)r used in %(use)s must be present in view but is missing.", @@ -2275,6 +2283,104 @@ class NameManager: ) view._raise_view_error(msg) + def combinate(groups): + # [['A'], ['B', 'C'], ['D', 'E']] + # returns + # [['A', 'B', 'D'], ['A', 'C', 'D'], ['A', 'B', 'E'], ['A', 'C', 'E']] + + # [['!A', '!B'], ['C', 'D']] + # returns + # [['!A', '!B', 'C'], ['!A', '!B', 'D']] + + # [['A', 'B'], ['!C', '!D']] + # returns + # [['A', '!C', '!D'], ['B', '!C', '!D']] + + # [['!A', '!B', 'C', 'D'], ['E', 'F']] + # returns + # [['!A', '!B', 'C', 'E'], ['!A', '!B', 'D', 'E'], ['!A', '!B', 'C', 'F'], ['!A', '!B', 'D', 'F']] + if not groups: + return [] + positive = [(group,) for group in groups[0] if not group or not group.startswith('!')] + negative = tuple(group for group in groups[0] if group and group.startswith('!')) + combinations = [negative + p for p in positive] if positive else [negative] + if len(groups) > 1: + combinations = [group + tuple(c) for c in combinate(groups[1:]) for group in combinations] + return [set(combination) for combination in set(tuple(combination) for combination in combinations)] + + for mandatory_for_groups, use in groups_uses.items(): + mandatory_combinations = combinate(mandatory_for_groups or [(None,)]) + available_combinations = [ + combination + for available_for_groups in self.available_fields[name]['groups'] + for combination in combinate(available_for_groups or [(None,)]) + ] + if ( + # If (None,) is in available_combinations, + # it means the field is in the view without any group restriction, + # and is therefore available for anyone / any group + {None} not in available_combinations + # If there are two mutually exclusive combinations, + # it means the field is available for anyone / any group + and not any( + {group[1:] if group.startswith('!') else '!' + group for group in combination} + in available_combinations + for combination in available_combinations + ) + # For all mandatory combinations, find an available combination + # which is included in the mandatory combination + # e.g. + # mandatory combination: A B C + # available combination: A B + # The above is valid, the field will be available for users having both A and B groups + # and the field is mandatory only for users having A B and C groups. + and not all( + any( + available_combination.issubset(mandatory_combination) + for available_combination in available_combinations + ) for mandatory_combination in mandatory_combinations + ) + # Same than above but take into account implied groups (e.g. group_system includes group_erp_manager and group_user) + # for positive groups + and not all( + any( + available_combination.issubset(combination) + for available_combination in available_combinations + for combination in + ( + mandatory_combination - {group} | {trans_group} + for group in mandatory_combination if group and not group.startswith('!') + for trans_group in view.env.ref(group).trans_implied_ids.get_external_id().values() + ) + ) + for mandatory_combination in mandatory_combinations + ) + # Same than above but take into account implied groups (e.g. group_system includes group_erp_manager and group_user) + # for negative groups + and not all( + any( + combination.issubset(mandatory_combination) + for available_combination in available_combinations + for combination in + ( + available_combination - {group} | {'!' + trans_group} + for group in available_combination if group and group.startswith('!') + for trans_group in view.env.ref(group[1:]).trans_implied_ids.get_external_id().values() + ) + ) for mandatory_combination in mandatory_combinations + ) + ): + msg = _( + "Field %(name)r used in %(use)s is restricted to the group(s) %(groups)s.", + name=name, use=use, groups=','.join( + group + for availability in self.available_fields[name]['groups'] + for combination in availability + for group in combination + ) + ) + view._raise_view_error(msg) + def update_available_fields(self): for name, info in self.available_fields.items(): info.update(self.field_info.get(name, ())) diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index d12d9ba65d7..35d12e23322 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -1308,7 +1308,7 @@ class GroupsView(models.Model): else: group_no_one = view.env.ref('base.group_no_one') group_employee = view.env.ref('base.group_user') - xml1, xml2, xml3 = [], [], [] + xml0, xml1, xml2, xml3 = [], [], [], [] xml_by_category = {} xml1.append(E.separator(string='User Type', colspan="2", groups='base.group_no_one')) @@ -1327,6 +1327,11 @@ class GroupsView(models.Model): if app.xml_id == 'base.module_category_user_type': # application name with a selection field field_name = name_selection_groups(gs.ids) + # test_reified_groups, put the user category type in invisible + # as it's used in domain of attrs of other fields, + # and the normal user category type field node is wrapped in a `groups="base.no_one"`, + # and is therefore removed when not in debug mode. + xml0.append(E.field(name=field_name, invisible="1", on_change="1")) user_type_field_name = field_name user_type_readonly = str({'readonly': [(user_type_field_name, '!=', group_employee.id)]}) attrs['widget'] = 'radio' @@ -1369,6 +1374,7 @@ class GroupsView(models.Model): xml2.append(E.group(*(xml_by_category[xml_cat]), col="2", string=master_category_name)) xml = E.field( + *(xml0), E.group(*(xml1), col="2", groups="base.group_no_one"), E.group(*(xml2), col="2", attrs=str(user_type_attrs)), E.group(*(xml3), col="4", attrs=str(user_type_attrs), groups="base.group_no_one"), name="groups_id", position="replace") diff --git a/odoo/addons/base/tests/test_acl.py b/odoo/addons/base/tests/test_acl.py index 9c10c50b1ca..0e7b1bf7873 100644 --- a/odoo/addons/base/tests/test_acl.py +++ b/odoo/addons/base/tests/test_acl.py @@ -48,7 +48,13 @@ class TestACL(TransactionCaseWithUserDemo): # Verify the test environment first original_fields = currency.fields_get([]) - form_view = currency.get_view(False, 'form') + with self.debug_mode(): + # + # + # + # + # + form_view = currency.get_view(False, 'form') view_arch = etree.fromstring(form_view.get('arch')) has_group_system = self.user_demo.has_group(GROUP_SYSTEM) self.assertFalse(has_group_system, "`demo` user should not belong to the restricted group before the test") @@ -74,7 +80,8 @@ class TestACL(TransactionCaseWithUserDemo): self.erp_system_group.users += self.user_demo has_group_system = self.user_demo.has_group(GROUP_SYSTEM) fields = currency.fields_get([]) - form_view = currency.get_view(False, 'form') + with self.debug_mode(): + form_view = currency.get_view(False, 'form') view_arch = etree.fromstring(form_view.get('arch')) self.assertTrue(has_group_system, "`demo` user should now belong to the restricted group") self.assertIn('decimal_places', fields, "'decimal_places' field must be properly visible again") diff --git a/odoo/addons/base/tests/test_views.py b/odoo/addons/base/tests/test_views.py index 8fa75f9fa8e..1f2acb4e8d9 100644 --- a/odoo/addons/base/tests/test_views.py +++ b/odoo/addons/base/tests/test_views.py @@ -2340,6 +2340,481 @@ class TestViews(ViewCase): """Field 'model' used in attrs ({'readonly': [('parent.model', '=', 'ir.ui.view')]}) must be present in view but is missing.""", ) + def test_attrs_groups_behavior(self): + view = self.View.create({ + 'name': 'foo', + 'model': 'res.partner', + 'arch': """ + + + +
+
+ + """, + }) + user_demo = self.env.ref('base.user_demo') + # Make sure demo doesn't have the base.group_system + self.assertFalse(self.env['res.partner'].with_user(user_demo).env.user.has_group('base.group_system')) + arch = self.env['res.partner'].with_user(user_demo).get_view(view_id=view.id)['arch'] + tree = etree.fromstring(arch) + self.assertTrue(tree.xpath('//field[@name="name"]')) + self.assertFalse(tree.xpath('//field[@name="company_id"]')) + self.assertTrue(tree.xpath('//div[@id="foo"]')) + self.assertFalse(tree.xpath('//div[@id="bar"]')) + + user_admin = self.env.ref('base.user_admin') + # Make sure admin has the base.group_system + self.assertTrue(self.env['res.partner'].with_user(user_admin).env.user.has_group('base.group_system')) + arch = self.env['res.partner'].with_user(user_admin).get_view(view_id=view.id)['arch'] + tree = etree.fromstring(arch) + self.assertTrue(tree.xpath('//field[@name="name"]')) + self.assertTrue(tree.xpath('//field[@name="company_id"]')) + self.assertTrue(tree.xpath('//div[@id="foo"]')) + self.assertTrue(tree.xpath('//div[@id="bar"]')) + + def test_attrs_groups_validation(self): + def validate(arch, valid=False, parent=False): + parent = 'parent.' if parent else '' + if valid: + self.assertValid(arch % {'attrs': f"""attrs="{{'invisible': [('{parent}name', '=', 'foo')]}}" """}) + self.assertValid(arch % {'attrs': f"""domain="[('name', '!=', {parent}name)]" """}) + self.assertValid(arch % {'attrs': f"""context="{{'default_name': {parent}name}}" """}) + self.assertValid(arch % {'attrs': f"""decoration-info="{parent}name == 'foo'" """}) + else: + self.assertInvalid( + arch % {'attrs': f"""attrs="{{'invisible': [('{parent}name', '=', 'foo')]}}" """}, + f"""Field 'name' used in attrs ({{'invisible': [('{parent}name', '=', 'foo')]}}) is restricted to the group(s)""", + ) + self.assertInvalid( + arch % {'attrs': f"""domain="[('name', '!=', {parent}name)]" """}, + f"""Field 'name' used in domain of ([('name', '!=', {parent}name)]) is restricted to the group(s)""", + ) + self.assertInvalid( + arch % {'attrs': f"""context="{{'default_name': {parent}name}}" """}, + f"""Field 'name' used in context ({{'default_name': {parent}name}}) is restricted to the group(s)""", + ) + self.assertInvalid( + arch % {'attrs': f"""decoration-info="{parent}name == 'foo'" """}, + f"""Field 'name' used in decoration-info={parent}name == 'foo' is restricted to the group(s)""", + ) + + + # Assert using a field restricted to a group + # in another field without the same group is invalid + validate(""" +
+ + + + """, valid=False) + + # Assert using a parent field restricted to a group + # in a child field without the same group is invalid + validate(""" +
+ + + + + + + + """, valid=False, parent=True) + + # Assert using a parent field restricted to a group + # in a child field with the same group is valid + validate(""" +
+ + + + + + + + """, valid=True, parent=True) + + # Assert using a parent field available for everyone + # in a child field restricted to a group is valid + validate(""" +
+ + + + + + + + """, valid=True, parent=True) + + # Assert using a field available for everyone + # in another field restricted to a group is valid + validate(""" +
+ + + + """, valid=True) + + # Assert using a field restricted to a group + # in another field with the same group is valid + validate(""" +
+ + + + """, valid=True) + + # Assert using a field available twice for 2 diffent groups + # in another field restricted to one of the 2 groups is valid + validate(""" +
+ + + + + """, valid=True) + + # Assert using a field restricted to a group only + # in other fields restricted to at least one different group is invalid + validate(""" +
+ + + + + """, valid=False) + + # Assert using a field available twice for 2 different groups + # in other fields restricted to the same 2 group is valid + validate(""" +
+ + + + + + """, valid=True) + + # Assert using a field available for 2 diffent groups, + # in another field restricted to one of the 2 groups is valid + validate(""" +
+ + + + """, valid=True) + + # Assert using a field available for 1 group only + # in another field restricted 2 groups is invalid + validate(""" +
+ + + + """, valid=False) + + # Assert using a field restricted to a group + # in another field restricted to a group including the group for which the field is available is valid + validate(""" +
+ + + + """, valid=True) + + # Assert using a parent field restricted to a group + # in a child field restricted to a group including the group for which the field is available is valid + validate(""" +
+ + + + + + + + """, valid=True, parent=True) + + # Assert using a field restricted to a group + # in another field restricted to a group not including the group for which the field is available is invalid + validate(""" +
+ + + + """, valid=False) + + # Assert using a parent field restricted to a group + # in a child field restricted to a group not including the group for which the field is available is invalid + validate(""" +
+ + + + + + + + """, valid=False, parent=True) + + # Assert using a field within a block restricted to a group + # in another field not restricted to the same group is invalid + validate(""" +
+ + + + + + """, valid=False) + + # Assert using a field within a block restricted to a group + # in another field within the same block restricted to a group is valid + validate(""" +
+ + + + +
+ """, valid=True) + + # Assert using a field within a block restricted to a group + # in another field within the same block restricted to a group and additional groups on the field node is valid + validate(""" +
+ + + + +
+ """, valid=True) + + # Assert using a field within a block restricted to a group + # in another field within a block restricted to the same group is valid + validate(""" +
+ + + + + + +
+ """, valid=True) + + # Assert using a field within a block restricted to a group + # in another field within a block restricted to a group including the group for which the field is available + # is valid + validate(""" +
+ + + + + + +
+ """, valid=True) + + # Assert using a field within a block restricted to a group + # in another field within a block restricted to a group not including the group for which the field is available + # is invalid + validate(""" +
+ + + + + + +
+ """, valid=False) + + # Assert using a parent field restricted to a group + # in a child field under a relational field restricted to the same group is valid + validate(""" +
+ + + + + + + + """, valid=True, parent=True) + + # Assert using a parent field restricted to a group + # in a child field under a relational field restricted + # to a group including the group for which the field is available is valid + validate(""" +
+ + + + + + + + """, valid=True, parent=True) + + # Assert using a parent field restricted to a group + # in a child field under a relational field restricted + # to a group not including the group for which the field is available is invalid + validate(""" +
+ + + + + + + + """, valid=False, parent=True) + + # Assert using a field restricted to users not having a group + # in another field not restricted to any group is invalid + validate(""" +
+ + + + """, valid=False) + + # Assert using a field not restricted to any group + # in another field restricted to users not having a group is valid + validate(""" +
+ + + + """, valid=True) + + # Assert using a field restricted to users not having multiple groups + # in another field restricted to users not having one of the group only is invalid + # e.g. + # if the user is portal, the field "name" will not be in the view + # but the field "inherit_id" where "name" is used will be in the view + # making it invalid. + validate(""" +
+ + + + """, valid=False) + + # Assert using a field restricted to users not having a group + # in another field restricted to users not having multiple group including the one above is valid + # e.g. + # if the user is portal, the field "name" will be in the view + # but the field "inherit_id" where "name" is used will not be in the view + # making it valid. + validate(""" +
+ + + + """, valid=True) + + # Assert using a field restricted to a non group + # in another field for which the non group is not implied is invalid + # e.g. + # if the user is employee, the field "name" will not be in the view + # but the field "inherit_id" where "name" is used will be in the view, + # making it invalid. + validate(""" +
+ + + + """, valid=False) + + # Assert using a field restricted to a non group + # in another field restricted to a non group implied in the non group of the available field is valid + # e.g. + # if the user is employee, the field "name" will be in the view + # but the field "inherit_id", where "name" is used, will not be in the view, + # therefore making it valid + validate(""" +
+ + + + """, valid=True) + + # Assert using a field restricted to non-admins, itself in a block restricted to employees, + # in another field restricted to a block restricted to employees + # is invalid + # e.g. + # if the user is admin, the field "name" will not be in the view + # but the field "inherit_id", where "name" is used, will be in the view, + # threfore making it invalid + validate(""" +
+ + + + + + +
+ """, valid=False) + + # Assert using a field restricted to a group + # in another field restricted the opposite group is invalid + # e.g. + # if the user is admin, the field "name" will be in the view + # but the field "inherit_id", where "name" is used, will not be in the view, + # therefore making it invalid + validate(""" +
+ + + + """, valid=False) + + # Assert having two times the same field with a mutually exclusive group + # and using that field in another field without any group is valid + validate(""" +
+ + + + + """, valid=True) + + # Assert having two times the same field with a mutually exclusive group + # and using that field in another field using the group is valid + validate(""" +
+ + + + + """, valid=True) + + # Assert having two times the same field with a mutually exclusive group + # and using that field in another field using the !group is valid + validate(""" +
+ + + + + """, valid=True) + + # Assert having two times the same field with a mutually exclusive group + # and using that field in another field restricted to any other group is valid + validate(""" +
+ + + + + """, valid=True) + def test_button(self): arch = """
diff --git a/odoo/addons/base/views/ir_model_views.xml b/odoo/addons/base/views/ir_model_views.xml index 2d2295c69fb..f216db0657c 100644 --- a/odoo/addons/base/views/ir_model_views.xml +++ b/odoo/addons/base/views/ir_model_views.xml @@ -248,6 +248,7 @@ ir.model.fields + diff --git a/odoo/addons/base/views/ir_module_views.xml b/odoo/addons/base/views/ir_module_views.xml index bd339520d2d..5aeaeda9add 100644 --- a/odoo/addons/base/views/ir_module_views.xml +++ b/odoo/addons/base/views/ir_module_views.xml @@ -70,6 +70,9 @@ By
+ + +