From 20a4c0ab864bc3d012db8a87e7e0728c81ff97b0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Iv=C3=A1n=20Todorovich?= <ivan.todorovich@gmail.com>
Date: Fri, 1 Jun 2018 16:47:50 -0300
Subject: [PATCH] [9.0] Improve document_page_approval (#163)

---
 document_page_approval/__openerp__.py         |   3 +-
 document_page_approval/models/__init__.py     |   2 +-
 ...ment_page_approval.py => document_page.py} |  33 ++++--
 ...y_workflow.py => document_page_history.py} |  68 +++++++++---
 .../security/document_page_security.xml       |  22 ++++
 .../tests/test_document_page_approval.py      |  14 +--
 .../views/document_page_approval.xml          |  54 ++++++---
 .../workflows/document_page_approval.xml      | 103 ------------------
 8 files changed, 146 insertions(+), 153 deletions(-)
 rename document_page_approval/models/{document_page_approval.py => document_page.py} (82%)
 rename document_page_approval/models/{document_page_history_workflow.py => document_page_history.py} (70%)
 delete mode 100644 document_page_approval/workflows/document_page_approval.xml

diff --git a/document_page_approval/__openerp__.py b/document_page_approval/__openerp__.py
index 114a9fd5..d68e627e 100644
--- a/document_page_approval/__openerp__.py
+++ b/document_page_approval/__openerp__.py
@@ -21,7 +21,7 @@
 
 {
     'name': 'Document Page Approval',
-    'version': '9.0.2.0.0',
+    'version': '9.0.2.1.0',
     "author": "Savoir-faire Linux,Odoo Community Association (OCA)",
     "website": "http://www.savoirfairelinux.com",
     "license": "AGPL-3",
@@ -32,7 +32,6 @@
     ],
     'data': [
         'data/email_template.xml',
-        'workflows/document_page_approval.xml',
         'views/document_page_approval.xml',
         'security/document_page_security.xml',
         'security/ir.model.access.csv',
diff --git a/document_page_approval/models/__init__.py b/document_page_approval/models/__init__.py
index 91075fe8..895435cf 100644
--- a/document_page_approval/models/__init__.py
+++ b/document_page_approval/models/__init__.py
@@ -1,2 +1,2 @@
 # -*- coding: utf-8 -*-
-from . import document_page_approval, document_page_history_workflow
+from . import document_page, document_page_history
diff --git a/document_page_approval/models/document_page_approval.py b/document_page_approval/models/document_page.py
similarity index 82%
rename from document_page_approval/models/document_page_approval.py
rename to document_page_approval/models/document_page.py
index 6e00fe3d..0856aefe 100644
--- a/document_page_approval/models/document_page_approval.py
+++ b/document_page_approval/models/document_page.py
@@ -23,7 +23,7 @@ from openerp import models, fields, api
 from ast import literal_eval
 
 
-class DocumentPageApproval(models.Model):
+class DocumentPage(models.Model):
     """Useful to know the state of a document."""
 
     _inherit = 'document.page'
@@ -83,6 +83,11 @@ class DocumentPageApproval(models.Model):
         string='Has changes pending approval'
     )
 
+    user_has_drafts = fields.Boolean(
+        compute='_compute_user_has_drafts',
+        string='User has drafts?',
+    )
+
     @api.multi
     @api.depends('approval_required', 'parent_id.is_approval_required')
     def _compute_is_approval_required(self):
@@ -117,14 +122,17 @@ class DocumentPageApproval(models.Model):
         # if it's not required, anyone can approve
         if not self.is_approval_required:
             return True
-        # to approve, you must have approver rights
-        approver_group_id = self.env.ref(
-            'document_page_approval.group_document_approver_user')
-        if approver_group_id not in user.groups_id:
+        # if user belongs to 'Knowledge / Manager', he can approve anything
+        if user.has_group('document_page.group_document_manager'):
+            return True
+        # to approve, user must have approver rights
+        if not user.has_group(
+                'document_page_approval.group_document_approver_user'):
             return False
-        # and belong to at least one of the approver_groups (if any is set)
+        # if there aren't any approver_groups_defined, user can approve
         if not self.approver_group_ids:
             return True
+        # to approve, user must belong to any of the approver groups
         return len(user.groups_id & self.approver_group_ids) > 0
 
     @api.multi
@@ -136,10 +144,19 @@ class DocumentPageApproval(models.Model):
                 ('state', '=', 'to approve')])
             rec.has_changes_pending_approval = (changes > 0)
 
+    @api.multi
+    def _compute_user_has_drafts(self):
+        history = self.env['document.page.history']
+        for rec in self:
+            changes = history.search_count([
+                ('page_id', '=', rec.id),
+                ('state', '=', 'draft')])
+            rec.user_has_drafts = (changes > 0)
+
     @api.multi
     def _create_history(self, vals):
-        res = super(DocumentPageApproval, self)._create_history(vals)
-        res.signal_workflow('document_page_auto_confirm')
+        res = super(DocumentPage, self)._create_history(vals)
+        res.action_to_approve()
 
     @api.multi
     def action_changes_pending_approval(self):
diff --git a/document_page_approval/models/document_page_history_workflow.py b/document_page_approval/models/document_page_history.py
similarity index 70%
rename from document_page_approval/models/document_page_history_workflow.py
rename to document_page_approval/models/document_page_history.py
index e66e05e1..90c64057 100644
--- a/document_page_approval/models/document_page_history_workflow.py
+++ b/document_page_approval/models/document_page_history.py
@@ -19,13 +19,12 @@
 #
 ##############################################################################
 
-from datetime import datetime
 from openerp.tools.translate import _
-from openerp.tools import DEFAULT_SERVER_DATETIME_FORMAT
 from openerp import models, fields, api
+from openerp.exceptions import UserError
 
 
-class DocumentPageHistoryWorkflow(models.Model):
+class DocumentPageHistory(models.Model):
     """Useful to manage edition's workflow on a document."""
 
     _name = 'document.page.history'
@@ -37,6 +36,7 @@ class DocumentPageHistoryWorkflow(models.Model):
         ('approved', 'Approved'),
         ('cancelled', 'Cancelled')],
         'Status',
+        default='draft',
         readonly=True,
     )
 
@@ -59,7 +59,8 @@ class DocumentPageHistoryWorkflow(models.Model):
     )
 
     am_i_approver = fields.Boolean(
-        related='page_id.am_i_approver'
+        related='page_id.am_i_approver',
+        related_sudo=False,
     )
 
     page_url = fields.Text(
@@ -68,37 +69,66 @@ class DocumentPageHistoryWorkflow(models.Model):
     )
 
     @api.multi
-    def page_approval_draft(self):
+    def action_draft(self):
         """Set a change request as draft"""
-        self.write({'state': 'draft'})
+        for rec in self:
+            if not rec.state == 'cancelled':
+                raise UserError(
+                    _('You need to cancel it before reopening.'))
+            if not (rec.am_i_owner or rec.am_i_approver):
+                raise UserError(
+                    _('You are not authorized to do this.\r\n'
+                      'Only owners or approvers can reopen Change Requests.'))
+            rec.write({'state': 'draft'})
 
     @api.multi
-    def page_approval_to_approve(self):
+    def action_to_approve(self):
         """Set a change request as to approve"""
-        self.write({'state': 'to approve'})
         template = self.env.ref(
             'document_page_approval.email_template_new_draft_need_approval')
         approver_gid = self.env.ref(
             'document_page_approval.group_document_approver_user')
         for rec in self:
+            if rec.state != 'draft':
+                raise UserError(
+                    _("Can't approve pages in '%s' state.") % rec.state)
+            if not (rec.am_i_owner or rec.am_i_approver):
+                raise UserError(
+                    _('You are not authorized to do this.\r\n'
+                      'Only owners or approvers can request approval.'))
+            # request approval
             if rec.is_approval_required:
+                rec.write({'state': 'to approve'})
                 guids = [g.id for g in rec.page_id.approver_group_ids]
                 users = self.env['res.users'].search([
                     ('groups_id', 'in', guids),
                     ('groups_id', 'in', approver_gid.id)])
                 rec.message_subscribe_users([u.id for u in users])
                 rec.message_post_with_template(template.id)
+            else:
+                # auto-approve if approval is not required
+                rec.action_approve()
 
     @api.multi
-    def page_approval_approved(self):
+    def action_approve(self):
         """Set a change request as approved."""
-        self.write({
-            'state': 'approved',
-            'approved_date': datetime.now().strftime(
-                DEFAULT_SERVER_DATETIME_FORMAT),
-            'approved_uid': self.env.uid
-        })
         for rec in self:
+            if rec.state not in ['draft', 'to approve']:
+                raise UserError(
+                    _("Can't approve page in '%s' state.") % rec.state)
+            if not rec.am_i_approver:
+                raise UserError(_(
+                    'You are not authorized to do this.\r\n'
+                    'Only approvers with these groups can approve this: '
+                    ) % ', '.join(
+                        [g.display_name
+                            for g in rec.page_id.approver_group_ids]))
+            # Update state
+            rec.write({
+                'state': 'approved',
+                'approved_date': fields.datetime.now(),
+                'approved_uid': self.env.uid,
+            })
             # Trigger computed field update
             rec.page_id._compute_history_head()
             # Notify state change
@@ -117,7 +147,7 @@ class DocumentPageHistoryWorkflow(models.Model):
             )
 
     @api.multi
-    def page_approval_cancelled(self):
+    def action_cancel(self):
         """Set a change request as cancelled."""
         self.write({'state': 'cancelled'})
         for rec in self:
@@ -128,6 +158,12 @@ class DocumentPageHistoryWorkflow(models.Model):
                     ) % (rec.display_name, self.env.user.name)
                 )
 
+    @api.multi
+    def action_cancel_and_draft(self):
+        """Set a change request as draft, cancelling it first"""
+        self.action_cancel()
+        self.action_draft()
+
     @api.multi
     def _compute_am_i_owner(self):
         """Check if current user is the owner"""
diff --git a/document_page_approval/security/document_page_security.xml b/document_page_approval/security/document_page_security.xml
index 3618a677..ad1328e8 100644
--- a/document_page_approval/security/document_page_security.xml
+++ b/document_page_approval/security/document_page_security.xml
@@ -12,5 +12,27 @@
         <field name="implied_ids" eval="[(4, ref('group_document_approver_user'))]"/>
     </record>
 
+
+    <record model="ir.rule" id="rule_change_request_global">
+        <field name="name">Change Request Global</field>
+        <field name="model_id" ref="model_document_page_history"/>
+        <field name="domain_force">['|',('state','=','approved'),('create_uid','=',user.id)]</field>
+        <field name="perm_read" eval="True"/>
+        <field name="perm_write" eval="True"/>
+        <field name="perm_unlink" eval="True"/>
+        <field name="perm_create" eval="True"/>
+    </record>
+
+    <record model="ir.rule" id="rule_change_request_approver">
+        <field name="name">Change Request Approver</field>
+        <field name="model_id" ref="model_document_page_history"/>
+        <field name="groups" eval="[(6, 0, [ref('group_document_approver_user')])]"/>
+        <field name="domain_force">[('state','!=','draft')]</field>
+        <field name="perm_read" eval="True"/>
+        <field name="perm_write" eval="True"/>
+        <field name="perm_unlink" eval="True"/>
+        <field name="perm_create" eval="True"/>
+    </record>
+
 </data>
 </odoo>
diff --git a/document_page_approval/tests/test_document_page_approval.py b/document_page_approval/tests/test_document_page_approval.py
index f418fd5d..2d3aa1a0 100644
--- a/document_page_approval/tests/test_document_page_approval.py
+++ b/document_page_approval/tests/test_document_page_approval.py
@@ -48,7 +48,7 @@ class TestDocumentPageApproval(common.TransactionCase):
         self.assertTrue(chreq.am_i_approver)
 
         # approve
-        chreq.signal_workflow('page_approval_approve')
+        chreq.action_approve()
         self.assertEqual(chreq.state, 'approved')
         self.assertEqual(chreq.content, page.content)
 
@@ -59,7 +59,7 @@ class TestDocumentPageApproval(common.TransactionCase):
             ('page_id', '=', page.id),
             ('state', '!=', 'approved')
         ])[0]
-        chreq.signal_workflow('page_approval_approve')
+        chreq.action_approve()
         self.assertEqual(page.content, 'New content')
 
     def test_change_request_auto_approve(self):
@@ -75,7 +75,7 @@ class TestDocumentPageApproval(common.TransactionCase):
         self.history_obj.search([
             ('page_id', '=', page.id),
             ('state', '!=', 'approved')
-        ]).signal_workflow('page_approval_approve')
+        ]).action_approve()
 
         # new change request from scrath
         chreq = self.history_obj.create({
@@ -89,25 +89,25 @@ class TestDocumentPageApproval(common.TransactionCase):
         self.assertNotEqual(page.approved_date, chreq.approved_date)
         self.assertNotEqual(page.approved_uid, chreq.approved_uid)
 
-        chreq.signal_workflow('page_approval_to_approve')
+        chreq.action_to_approve()
         self.assertEqual(chreq.state, 'to approve')
         self.assertNotEqual(page.content, chreq.content)
         self.assertNotEqual(page.approved_date, chreq.approved_date)
         self.assertNotEqual(page.approved_uid, chreq.approved_uid)
 
-        chreq.signal_workflow('page_approval_cancel')
+        chreq.action_cancel()
         self.assertEqual(chreq.state, 'cancelled')
         self.assertNotEqual(page.content, chreq.content)
         self.assertNotEqual(page.approved_date, chreq.approved_date)
         self.assertNotEqual(page.approved_uid, chreq.approved_uid)
 
-        chreq.signal_workflow('page_approval_reopen')
+        chreq.action_draft()
         self.assertEqual(chreq.state, 'draft')
         self.assertNotEqual(page.content, chreq.content)
         self.assertNotEqual(page.approved_date, chreq.approved_date)
         self.assertNotEqual(page.approved_uid, chreq.approved_uid)
 
-        chreq.signal_workflow('page_approval_approve')
+        chreq.action_approve()
         self.assertEqual(chreq.state, 'approved')
         self.assertEqual(page.content, chreq.content)
         self.assertEqual(page.approved_date, chreq.approved_date)
diff --git a/document_page_approval/views/document_page_approval.xml b/document_page_approval/views/document_page_approval.xml
index 60a8176e..59bc9818 100644
--- a/document_page_approval/views/document_page_approval.xml
+++ b/document_page_approval/views/document_page_approval.xml
@@ -10,20 +10,23 @@
                 <sheet position="before">
                     <header>
                         <!-- draft -> to approve -->
-                        <button name="page_approval_to_approve" string="Send to Review" class="oe_highlight"
+                        <button name="action_to_approve" type="object" string="Send to Review" class="oe_highlight"
                                 attrs="{'invisible':['|','|',('is_approval_required','=',False),('am_i_owner','=',False),('state', 'not in', ['draft'])]}"/>
                         <!-- approve if i am approver -->
-                        <button name="page_approval_approve" string="Approve" class="oe_highlight"
+                        <button name="action_approve" type="object" string="Approve" class="oe_highlight"
                                 attrs="{'invisible':['|','|',('is_approval_required','=',False),('am_i_approver','=',False),('state','not in',['draft','to approve'])]}"/>
                         <!-- approve if it's not required and i am owner -->
-                        <button name="page_approval_approve" string="Approve" class="oe_highlight"
+                        <button name="action_approve" type="object" string="Approve" class="oe_highlight"
                                 attrs="{'invisible':['|','|',('is_approval_required','=',True),('am_i_owner','=',False),('state','not in',['draft', 'to approve'])]}"/>
                         <!-- cancel if i am owner or approver -->
-                        <button name="page_approval_cancel" string="Cancel"
+                        <button name="action_cancel" type="object" string="Cancel"
                                 attrs="{'invisible':['|','&amp;',('am_i_owner','=',False),('am_i_approver','=',False),('state','not in',['draft','to approve'])]}"/>
                         <!-- reopen if i am owner or approver -->
-                        <button name="page_approval_reopen" string="Back to draft"
+                        <button name="action_draft" type="object" string="Back to draft"
                                 attrs="{'invisible':['|','&amp;',('am_i_owner','=',False),('am_i_approver','=',False),('state','not in',['cancelled'])]}"/>
+                        <!-- cancel & reopen, if i am owner or approver -->
+                        <button name="action_draft" type="object" string="Back to draft"
+                                attrs="{'invisible':['|','&amp;',('am_i_owner','=',False),('am_i_approver','=',False),('state','not in',['to approve'])]}"/>
                         <field name="am_i_owner" invisible="1"/>
                         <field name="am_i_approver" invisible="1"/>
                         <field name="is_approval_required" invisible="1"/>
@@ -36,9 +39,12 @@
                         <field name="approved_date" readonly="1" attrs="{'invisible':[('state','not in',['approved'])]}"/>
                     </group>
                 </xpath>
-                <field name="content" position="attributes">
-                    <attribute name="attrs">{'readonly': [('state', 'not in', ['draft'])]}</attribute>
-                </field>
+                <!-- Readonly fields -->
+                <field name="content" position="attributes"><attribute name="attrs">{'readonly': [('state', 'not in', ['draft'])]}</attribute></field>
+                <field name="page_id" position="attributes"><attribute name="attrs">{'readonly': [('state', 'not in', ['draft'])]}</attribute></field>
+                <field name="name"    position="attributes"><attribute name="attrs">{'readonly': [('state', 'not in', ['draft'])]}</attribute></field>
+                <field name="summary" position="attributes"><attribute name="attrs">{'readonly': [('state', 'not in', ['draft'])]}</attribute></field>
+                <!-- Chatter -->
                 <sheet position="after">
                     <div class="oe_chatter">
                         <field name="message_follower_ids" widget="mail_followers"/>
@@ -48,6 +54,19 @@
             </field>
         </record>
 
+        <!-- History Form View Manager Rights -->
+        <record id="wiki_history_form_inherit_manager" model="ir.ui.view">
+            <field name="name">document.page.history.form</field>
+            <field name="model">document.page.history</field>
+            <field name="inherit_id" ref="wiki_history_form_inherit"/>
+            <field name="groups_id" eval="[(6, 0, [ref('document_page.group_document_manager')])]"/>
+            <field name="arch" type="xml">
+                <!-- Readonly fields -->
+                <field name="name"    position="attributes"><attribute name="attrs">{'readonly': False}</attribute></field>
+                <field name="summary" position="attributes"><attribute name="attrs">{'readonly': False}</attribute></field>
+            </field>
+        </record>
+
         <!-- Page Form View -->
         <record id="wiki_form_inherit" model="ir.ui.view">
             <field name="name">document.page.form</field>
@@ -64,17 +83,20 @@
                          attrs="{'invisible': [('is_approval_required','=',False)]}">
                         This document requires approval. If edited, you will create a new <b>Change Request</b>.
                     </div>
+                    <div class="alert alert-warning oe_edit_only" role="alert" style="margin-bottom:0px;"
+                         attrs="{'invisible': [('user_has_drafts','=',False)]}">
+                        <b>You already have a Draft Change Request for this page.</b>
+                        It is highly recommended that you edit that one instead of creating a new one.
+                    </div>
                     <field name="is_approval_required" invisible="1"/>
                     <field name="has_changes_pending_approval" invisible="1"/>
+                    <field name="user_has_drafts" invisible="1"/>
                 </sheet>
 
-                <field name="type" position="before">
-                    <div class="oe_button_box" name="button_box">
-                        <button class="oe_stat_button" name="action_changes_pending_approval"
-                                string="Change Requests" type="object"
-                                attrs="{'invisible':[('has_changes_pending_approval','=',False)]}" icon="fa-edit"/>
-                    </div>
-                </field>
+                <button name="toggle_active" position="after">
+                    <button class="oe_stat_button" name="action_changes_pending_approval" string="Change Requests" type="object"
+                            attrs="{'invisible':[('has_changes_pending_approval','=',False),('user_has_drafts','=',False)]}" icon="fa-edit"/>
+                </button>
 
                 <field name="content_uid" position="after">
                     <field name="approved_uid"/>
@@ -176,7 +198,7 @@
             <field name="res_model">document.page.history</field>
             <field name="view_type">form</field>
             <field name="view_mode">tree,form</field>
-            <field name="context">{'search_default_state':'to approve'}</field>
+            <field name="context">{'search_default_draft': 1, 'search_default_pending': 1}</field>
         </record>
 
         <menuitem id="menu_page_change_requests"
diff --git a/document_page_approval/workflows/document_page_approval.xml b/document_page_approval/workflows/document_page_approval.xml
deleted file mode 100644
index 81a5ba85..00000000
--- a/document_page_approval/workflows/document_page_approval.xml
+++ /dev/null
@@ -1,103 +0,0 @@
-<?xml version="1.0"?>
-<odoo>
-        <record model="workflow" id="wkf_document_page_history_aproval">
-            <field name="name">document.page.history.aproval.wkf</field>
-            <field name="osv">document.page.history</field>
-            <field name="on_create">True</field>
-        </record>
-
-        <record model="workflow.activity" id="act_draft">
-            <field name="wkf_id" ref="wkf_document_page_history_aproval" />
-            <field name="flow_start">True</field>
-            <field name="name">draft</field>
-            <field name="kind">function</field>
-            <field name="action">page_approval_draft()</field>
-        </record>
-
-        <record model="workflow.activity" id="act_to_approve">
-            <field name="wkf_id" ref="wkf_document_page_history_aproval" />
-            <field name="name">to approve</field>
-            <field name="kind">function</field>
-            <field name="action">page_approval_to_approve()</field>
-        </record>
-
-        <record model="workflow.activity" id="act_approved">
-            <field name="wkf_id" ref="wkf_document_page_history_aproval" />
-            <field name="name">approved</field>
-            <field name="kind">function</field>
-            <field name="action">page_approval_approved()</field>
-            <field name="flow_stop">True</field>
-        </record>
-
-        <record model="workflow.activity" id="act_cancelled">
-            <field name="wkf_id" ref="wkf_document_page_history_aproval" />
-            <field name="name">cancelled</field>
-            <field name="kind">function</field>
-            <field name="action">page_approval_cancelled()</field>
-        </record>
-
-        <!-- Transitions -->
-        <record model="workflow.transition" id="tdr">
-            <field name="act_from" ref="act_draft"/>
-            <field name="act_to" ref="act_to_approve"/>
-            <field name="condition">am_i_owner</field>
-            <field name="signal">page_approval_to_approve</field>
-        </record>
-
-        <record model="workflow.transition" id="tda">
-            <field name="act_from" ref="act_draft"/>
-            <field name="act_to" ref="act_approved"/>
-            <field name="condition">am_i_approver</field>
-            <field name="signal">page_approval_approve</field>
-        </record>
-
-        <record model="workflow.transition" id="tra">
-            <field name="act_from" ref="act_to_approve"/>
-            <field name="act_to" ref="act_approved"/>
-            <field name="condition">am_i_approver</field>
-            <field name="signal">page_approval_approve</field>
-        </record>
-
-        <record model="workflow.transition" id="tad">
-            <field name="act_from" ref="act_approved"/>
-            <field name="act_to" ref="act_draft"/>
-            <field name="condition">am_i_approver</field>
-            <field name="signal">edit</field>
-        </record>
-
-        <record model="workflow.transition" id="tdc">
-            <field name="act_from" ref="act_draft"/>
-            <field name="act_to" ref="act_cancelled"/>
-            <field name="condition">am_i_owner</field>
-            <field name="signal">page_approval_cancel</field>
-        </record>
-
-        <record model="workflow.transition" id="trc">
-            <field name="act_from" ref="act_to_approve"/>
-            <field name="act_to" ref="act_cancelled"/>
-            <field name="condition">am_i_owner or am_i_approver</field>
-            <field name="signal">page_approval_cancel</field>
-        </record>
-
-        <record model="workflow.transition" id="tcd">
-            <field name="act_from" ref="act_cancelled"/>
-            <field name="act_to" ref="act_draft"/>
-            <field name="condition">am_i_owner or am_i_approver</field>
-            <field name="signal">page_approval_reopen</field>
-        </record>
-
-        <!-- Automatic Transitions for change requests created directly from documents -->
-        <record model="workflow.transition" id="tda_auto">
-            <field name="act_from" ref="act_draft"/>
-            <field name="act_to" ref="act_approved"/>
-            <field name="condition">not is_approval_required</field>
-            <field name="signal">document_page_auto_confirm</field>
-        </record>
-
-        <record model="workflow.transition" id="tdr_auto">
-            <field name="act_from" ref="act_draft"/>
-            <field name="act_to" ref="act_to_approve"/>
-            <field name="condition">is_approval_required</field>
-            <field name="signal">document_page_auto_confirm</field>
-        </record>
-</odoo>