[ADD] review fixes

Author: gfcapalbo

auditlog_security/__manifest__.py

@@ -3,7 +3,7 @@
 
 {
     "name": "Audit Log Permissions",
-    "version": "11.0.1.0.1",
+    "version": "11.0.1.0.0",
     "author": "Therp B.V.,Odoo Community Association (OCA)",
     "license": "AGPL-3",
     "website": "https://github.com/OCA/server-tools/",
@@ -13,7 +13,6 @@
         "contacts",
     ],
     "data": [
-        "demo/auditlog_rule.xml",
         "views/auditlog_view.xml",
     ],
     "application": True,

auditlog_security/demo/auditlog_rule.xml

@@ -1,7 +1,7 @@
 # Copyright 2021 Therp B.V.
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
 
-from odoo import models, fields, api, modules, _
+from odoo import exceptions, models, fields, api, modules, _
 from odoo.addons.auditlog.models.rule import FIELDS_BLACKLIST
 
 
@@ -10,20 +10,36 @@ class AuditlogRule(models.Model):
 
     field_ids = fields.Many2many(
         "ir.model.fields",
-        required=True,
     )
     group_ids = fields.Many2many(
         "res.groups",
-        string="Groups",
+        default= lambda x: x.env.ref('base.group_user'),
         help="""Groups that will be allowed to see the logged fields, if left empty 
-                all groups will be allowed (global rule creation)""",
+                default will be all users with a login""",
     )
+    all_fields = fields.Boolean(
+            string="Publicly visible log lines for all fields",
+            readonly=True,
+            compute="compute_all_fields",
+            store=False,
+            help="Check this field if you want this model to be visible by everyone")
+
+    @api.depends('field_ids')
+    def compute_all_fields(self):
+        for this in self.sudo():
+            this.all_fields = not bool(len(this.field_ids))
+
+    @api.onchange('model_id')
+    def onchange_model_id(self):
+        # if model changes we must wipe out all field ids
+        self.field_ids = False
+
 
-    """ note this solution will work only with a hardcoded design of models,
-    because on initialization , self.model_id.id still is not defined.
-    for now, to keep generality we put the filtering in the view."""
 
     def get_field_ids_domain(self):
+        """ note this solution will work only with a hardcoded design of models,
+        because on initialization , self.model_id.id still is not defined.
+        for now, to keep generality we put the filtering in the view."""
         return [
             ("model_id", "=", self.env.ref("base.model_res_partner").id),
             ("name", "not in", FIELDS_BLACKLIST),
@@ -32,40 +48,63 @@ def get_field_ids_domain(self):
     def unlink(self):
         # if we delete auditlog rule, corresponding ir.rules are removed
         # TODO PROPOSAL: a warning here with detailed information?
+        to_delete = self.env["ir.rule"].with_context(auditlog_write=True).search(
+                [("auditlog_id", "=", self.ids)])
         res = super(AuditlogRule, self).unlink()
-        return (
-            res
-            and self.env["ir.rule"]
-            .with_context(auditlog_write=True)
-            .search([("auditlog_id", "in", self.ids)])
-            .unlink()
-        )
+        if res:
+            res = res and to_delete.unlink()
+        return res
 
+    @api.model
+    def create(self, vals):
+        if 'group_ids' not in vals:
+            # if group has been removed and no group specified ad base user default
+            vals['group_ids'] = [(6, 0, [self.env.ref('base.group_user').id])]
+        if 'field_ids' not in vals and not vals.get('all_fields'):
+            # this was done because of a 
+            # bug in attrs={'required': [('all_fields', '!=', True}
+            raise exceptions.ValidationError(
+                _("please specify fields to log or make all_fields true"))
+        res = super(AuditlogRule, self).create(vals)
+        res.generate_rules()
+        return res
+        
     @api.multi
     def write(self, vals):
         res = super(AuditlogRule, self).write(vals)
         for this in self:
-            if any([x in vals for x in ("group_ids", "field_ids", "model_id")]):
+            if not this.group_ids:
+                # if no group is specified, we will not create a global, we
+                # always put at least base.group_user
+                this.write(
+                    {'group_ids': [(6, 0, [
+                        self.env.ref('base.group_user').id])]})
+            if any([x in vals for x in (
+                    "group_ids", "field_ids", "model_id", "all_fields")]):
                 this.generate_rules()
         return res
 
     def generate_rules(self):
-        old_rule = self.env["ir.rule"].search([("auditlog_id", "=", self.id)], limit=1)
-        domain_force = (
-            "[ "
-            + " ('log_id.model_id' , '=', %s)," % (self.model_id.id)
-            + "('field_id', 'in',  %s)" % (self.field_ids.ids)
-            + "]"
-        )
-        values = {
+        old_rule = self.env["ir.rule"].search(
+                [("auditlog_id", "=", self.id)], limit=1)
+        values = self._prepare_rule_values()
+        if old_rule:
+            old_rule.with_context(auditlog_write=True).write(values)
+        else:
+            self.with_context(auditlog_write=True).env["ir.rule"].create(
+                    values)
+
+    def _prepare_rule_values(self):
+        domain_force = "[" + " ('log_id.model_id' , '=', %s)," % (
+                self.model_id.id)
+        if self.field_ids:
+            domain_force += "('field_id', 'in',  %s)" % (self.field_ids.ids)
+        domain_force += "]"
+        return {
             "name": "auditlog_extended_%s" % self.id,
             "model_id": self.env.ref("auditlog.model_auditlog_log_line").id,
             "groups": [(6, 0, self.group_ids.ids)],
             "perm_read": True,
             "domain_force": domain_force,
             "auditlog_id": self.id,
         }
-        if old_rule:
-            old_rule.with_context(auditlog_write=True).write(values)
-        else:
-            self.with_context(auditlog_write=True).env["ir.rule"].create(values)

auditlog_security/models/auditlog_rule.py

@@ -13,19 +13,6 @@ class IrRule(models.Model):
         help="Auditlog Rule that generated this ir.rule",
     )
 
-    def prevent_rule_mod(self, vals=None):
-        auditlog_write = self.env.context.get("auditlog_write")
-        if "auditlog_id" in vals and not auditlog_write:
-            raise exceptions.validationerror(_("""Cannot change auditlog_id"""))
-        for this in self:
-            if this.auditlog_id and not auditlog_write:
-                raise exceptions.validationerror(
-                    _(
-                        """
-                     auditlog line rules are automatically generated from the auditlog 
-                     interface, please use that to edit/delete/"""
-                    )
-                )
 
     @api.model
     def create(self, values):
@@ -35,18 +22,30 @@ def create(self, values):
             raise exceptions.ValidationError(
                 _(
                     """
-                 Auditlog line rules are automatically generated from the auditlog 
-                 interface, please use that to create"""
+                 Auditlog line rules are automatically generated from the  
+                 auditlog interface, please use that to create"""
                 )
             )
         return super(IrRule, self).create(values)
 
     @api.multi
     def write(self, vals):
-        self.prevent_rule_mod(vals)
+        if "auditlog_id" in vals and not self.env.context.get(
+                'auditlog_write'):
+            raise exceptions.ValidationError(_("""Cannot change auditlog_id"""))
         return super(IrRule, self).write(vals)
 
     @api.multi
     def unlink(self):
-        self.prevent_rule_mod()
+        auditlog_write = self.env.context.get("auditlog_write")
+        for this in self:
+            if this.auditlog_id and not auditlog_write:
+                raise exceptions.ValidationError(
+                    _(
+
+                    """
+                 Auditlog line rules are automatically generated from the  
+                 auditlog interface, please use that to delete"""
+                    )
+                )
         return super(IrRule, self).unlink()

auditlog_security/models/ir_rule.py

@@ -50,11 +50,11 @@
                     would be skipped anyway, so we repeat here to avoid incongruencies 
                     -->
                     <field name="field_ids"
-                        domain="[('model_id', '=' , model_id),('name', 'not in', ['id', 'create_uid', 'create_date', 'write_uid', 'write_date','display_name', '__last_update']) ]"
-                        />
+                        domain="[('model_id', '=' , model_id),('name', 'not in', ['id', 'create_uid', 'create_date', 'write_uid', 'write_date','display_name', '__last_update']) ]"/>
                 </group>
                 <group>
-                    <field name="group_ids"/>                         
+                    <field name="group_ids" /> 
+                    <field name="all_fields"/>
                 </group>
             </xpath>                                                                 
 
@@ -81,7 +81,7 @@
             <search string="Log Lines">
                 <filter name="has_extensions"
                     domain="['|', ('field_ids','!=', False), ('group_ids', '!=', False)]" 
-                    string="Has security Extensions"/>
+                    string="Has Security Extensions"/>
                 <filter name="global_security"
                     domain="[('field_ids','!=', False)]" 
                     string="Accessible to all - globally accessible"/>