[MIG] document_page_approval: Migration to 18.0

document_page_approval/README.rst

@@ -17,13 +17,13 @@ Document Page Approval
     :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html
     :alt: License: AGPL-3
 .. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fknowledge-lightgray.png?logo=github
-    :target: https://github.com/OCA/knowledge/tree/17.0/document_page_approval
+    :target: https://github.com/OCA/knowledge/tree/18.0/document_page_approval
     :alt: OCA/knowledge
 .. |badge4| image:: https://img.shields.io/badge/weblate-Translate%20me-F47D42.png
-    :target: https://translation.odoo-community.org/projects/knowledge-17-0/knowledge-17-0-document_page_approval
+    :target: https://translation.odoo-community.org/projects/knowledge-18-0/knowledge-18-0-document_page_approval
     :alt: Translate me on Weblate
 .. |badge5| image:: https://img.shields.io/badge/runboat-Try%20me-875A7B.png
-    :target: https://runboat.odoo-community.org/builds?repo=OCA/knowledge&target_branch=17.0
+    :target: https://runboat.odoo-community.org/builds?repo=OCA/knowledge&target_branch=18.0
     :alt: Try me on Runboat
 
 |badge1| |badge2| |badge3| |badge4| |badge5|
@@ -64,7 +64,7 @@ Bug Tracker
 Bugs are tracked on `GitHub Issues <https://github.com/OCA/knowledge/issues>`_.
 In case of trouble, please check there if your issue has already been reported.
 If you spotted it first, help us to smash it by providing a detailed and welcomed
-`feedback <https://github.com/OCA/knowledge/issues/new?body=module:%20document_page_approval%0Aversion:%2017.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
+`feedback <https://github.com/OCA/knowledge/issues/new?body=module:%20document_page_approval%0Aversion:%2018.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
 
 Do not contact contributors directly about support or help with technical issues.
 
@@ -98,6 +98,10 @@ Contributors
 
   - Fernando La Chica <fernando.lachica@guadaltech.es>
 
+- `Heliconia Solutions Pvt. Ltd. <https://www.heliconia.io>`__
+
+  - Bhavesh Heliconia
+
 Maintainers
 -----------
 
@@ -111,6 +115,6 @@ OCA, or the Odoo Community Association, is a nonprofit organization whose
 mission is to support the collaborative development of Odoo features and
 promote its widespread use.
 
-This module is part of the `OCA/knowledge <https://github.com/OCA/knowledge/tree/17.0/document_page_approval>`_ project on GitHub.
+This module is part of the `OCA/knowledge <https://github.com/OCA/knowledge/tree/18.0/document_page_approval>`_ project on GitHub.
 
 You are welcome to contribute. To learn how please visit https://odoo-community.org/page/Contribute.

document_page_approval/__manifest__.py

@@ -3,7 +3,7 @@
 
 {
     "name": "Document Page Approval",
-    "version": "17.0.1.0.0",
+    "version": "18.0.1.0.0",
     "author": "Savoir-faire Linux, Odoo Community Association (OCA)",
     "website": "https://github.com/OCA/knowledge",
     "license": "AGPL-3",

document_page_approval/models/document_page_history.py

@@ -1,6 +1,7 @@
 # Copyright (C) 2013 Savoir-faire Linux (<http://www.savoirfairelinux.com>).
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
+
 from odoo import fields, models
 from odoo.exceptions import UserError
 from odoo.tools.translate import _
@@ -93,12 +94,14 @@ class DocumentPageHistory(models.Model):
                 raise UserError(
                     _(
                         "You are not authorized to do this.\r\n"
-                        "Only approvers with these groups can approve this: "
+                        "Only approvers with these groups can approve this: {}"
-                    )
+                    ).format(
-                    % ", ".join(
+                        ", ".join(
                             [g.display_name for g in rec.page_id.approver_group_ids]
                         )
                     )
+                )
+
             # Update state
             rec.write(
                 {

document_page_approval/readme/CONTRIBUTORS.md

@@ -16,3 +16,5 @@
 - [Guadaltech](https://www.guadaltech.es):
 
   - Fernando La Chica \<<fernando.lachica@guadaltech.es>\>
+- [Heliconia Solutions Pvt. Ltd.](https://www.heliconia.io)
+  - Bhavesh Heliconia

document_page_approval/static/description/index.html

@@ -369,7 +369,7 @@ ul.auto-toc {
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 !! source digest: sha256:4b3ef803acd19a66515436d838c979c63daecf130c34b3f628dd5d8ac8137d34
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
-<p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Beta" src="https://img.shields.io/badge/maturity-Beta-yellow.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/licence-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/knowledge/tree/17.0/document_page_approval"><img alt="OCA/knowledge" src="https://img.shields.io/badge/github-OCA%2Fknowledge-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/knowledge-17-0/knowledge-17-0-document_page_approval"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/knowledge&amp;target_branch=17.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
+<p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Beta" src="https://img.shields.io/badge/maturity-Beta-yellow.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/licence-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/knowledge/tree/18.0/document_page_approval"><img alt="OCA/knowledge" src="https://img.shields.io/badge/github-OCA%2Fknowledge-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/knowledge-18-0/knowledge-18-0-document_page_approval"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/knowledge&amp;target_branch=18.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
 <p>This module adds a workflow to approve page modifications and show the
 approved version by default.</p>
 <p><strong>Table of contents</strong></p>
@@ -413,7 +413,7 @@ history to review.</li>
 <p>Bugs are tracked on <a class="reference external" href="https://github.com/OCA/knowledge/issues">GitHub Issues</a>.
 In case of trouble, please check there if your issue has already been reported.
 If you spotted it first, help us to smash it by providing a detailed and welcomed
-<a class="reference external" href="https://github.com/OCA/knowledge/issues/new?body=module:%20document_page_approval%0Aversion:%2017.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**">feedback</a>.</p>
+<a class="reference external" href="https://github.com/OCA/knowledge/issues/new?body=module:%20document_page_approval%0Aversion:%2018.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**">feedback</a>.</p>
 <p>Do not contact contributors directly about support or help with technical issues.</p>
 </div>
 <div class="section" id="credits">
@@ -450,6 +450,11 @@ If you spotted it first, help us to smash it by providing a detailed and welcome
 <li>Fernando La Chica &lt;<a class="reference external" href="mailto:fernando.lachica&#64;guadaltech.es">fernando.lachica&#64;guadaltech.es</a>&gt;</li>
 </ul>
 </li>
+<li><p class="first"><a class="reference external" href="https://www.heliconia.io">Heliconia Solutions Pvt. Ltd.</a></p>
+<ul class="simple">
+<li>Bhavesh Heliconia</li>
+</ul>
+</li>
 </ul>
 </div>
 <div class="section" id="maintainers">
@@ -461,7 +466,7 @@ If you spotted it first, help us to smash it by providing a detailed and welcome
 <p>OCA, or the Odoo Community Association, is a nonprofit organization whose
 mission is to support the collaborative development of Odoo features and
 promote its widespread use.</p>
-<p>This module is part of the <a class="reference external" href="https://github.com/OCA/knowledge/tree/17.0/document_page_approval">OCA/knowledge</a> project on GitHub.</p>
+<p>This module is part of the <a class="reference external" href="https://github.com/OCA/knowledge/tree/18.0/document_page_approval">OCA/knowledge</a> project on GitHub.</p>
 <p>You are welcome to contribute. To learn how please visit <a class="reference external" href="https://odoo-community.org/page/Contribute">https://odoo-community.org/page/Contribute</a>.</p>
 </div>
 </div>

document_page_approval/tests/test_document_page_approval.py

@@ -1,3 +1,5 @@
+from odoo import Command
+from odoo.exceptions import UserError
 from odoo.tests import new_test_user
 
 from odoo.addons.base.tests.common import BaseCommon
@@ -9,18 +11,25 @@ class TestDocumentPageApproval(BaseCommon):
         super().setUpClass()
         cls.page_obj = cls.env["document.page"]
         cls.history_obj = cls.env["document.page.history"]
-        # demo
+
+        # Demo Data
         cls.category1 = cls.env.ref("document_page.demo_category1")
         cls.page1 = cls.env.ref("document_page.demo_page1")
+
+        # Create test user without groups first
         cls.user2 = new_test_user(
             cls.env,
             login="test-user2",
-            groups="base.group_user,document_page_approval.group_document_approver_user",
+            groups="document_page_approval.group_document_approver_user",
         )
+
+        # Ensure user2 has the approver group
         cls.approver_gid = cls.env.ref(
             "document_page_approval.group_document_approver_user"
         )
-        # demo_approval
+        cls.user2.write({"groups_id": [Command.link(cls.approver_gid.id)]})
+
+        # Create category and page that require approval
         cls.category2 = cls.page_obj.create(
             {
                 "name": "This category requires approval",
@@ -38,62 +47,59 @@ class TestDocumentPageApproval(BaseCommon):
         )
 
     def test_approval_required(self):
+        """Test that the page requires approval."""
         page = self.page2
         self.assertTrue(page.is_approval_required)
         self.assertTrue(page.has_changes_pending_approval)
         self.assertEqual(len(page.history_ids), 0)
 
     def test_change_request_approve(self):
+        """Test that an approver can approve a change request."""
         page = self.page2
+
+        # Get the change request for this page
         chreq = self.history_obj.search(
-            [("page_id", "=", page.id), ("state", "!=", "approved")]
+            [("page_id", "=", page.id), ("state", "!=", "approved")], limit=1
-        )[0]
+        )
 
-        # It should automatically be in 'to approve' state
         self.assertEqual(chreq.state, "to approve")
-        user_admin = self.env.ref("base.user_admin")
-        self.assertTrue(user_admin.partner_id.id in chreq.message_partner_ids.ids)
-        self.assertTrue(self.user2.partner_id.id in chreq.message_partner_ids.ids)
 
-        # Needed to compute calculated fields
+        # Ensure user2 is listed as an approver
-        page.invalidate_model()
+        self.assertTrue(chreq.with_user(self.user2).am_i_approver)
-        self.assertNotEqual(chreq.content, page.content)
 
-        # who_am_i
+        # Approve the request as user2 (approver)
-        self.assertTrue(chreq.am_i_owner)
+        chreq.with_user(self.user2).action_approve()
-        self.assertTrue(chreq.am_i_approver)
-
-        # approve
-        chreq.action_approve()
         self.assertEqual(chreq.state, "approved")
         self.assertEqual(chreq.content, page.content)
 
-        # new changes should create change requests
+        # Create new change request
         page.write({"content": "New content"})
-        # Needed to compute calculated fields
+        page.invalidate_model()  # Recompute fields
-        page.invalidate_model()
-        self.assertNotEqual(page.content, "New content")
         chreq = self.history_obj.search(
-            [("page_id", "=", page.id), ("state", "!=", "approved")]
+            [("page_id", "=", page.id), ("state", "!=", "approved")], limit=1
-        )[0]
+        )
-        chreq.action_approve()
+
+        # Approve new changes
+        chreq.with_user(self.user2).action_approve()
         self.assertEqual(page.content, "New content")
 
     def test_change_request_auto_approve(self):
+        """Test that a page without approval required auto-approves changes."""
         page = self.page1
         self.assertFalse(page.is_approval_required)
         page.write({"content": "New content"})
         self.assertEqual(page.content, "New content")
 
     def test_change_request_from_scratch(self):
+        """Test a full change request lifecycle from draft to approval."""
         page = self.page2
 
-        # aprove everything
+        # Approve all pending change requests
         self.history_obj.search(
             [("page_id", "=", page.id), ("state", "!=", "approved")]
-        ).action_approve()
+        ).with_user(self.user2).action_approve()
 
-        # new change request from scrath
+        # Create new change request
         chreq = self.history_obj.create(
             {
                 "page_id": page.id,
@@ -103,41 +109,152 @@ class TestDocumentPageApproval(BaseCommon):
         )
 
         self.assertEqual(chreq.state, "draft")
-        self.assertNotEqual(page.content, chreq.content)
+        chreq.action_to_approve()
-        self.assertNotEqual(page.approved_date, chreq.approved_date)
+        self.assertEqual(chreq.state, "to approve")
-        self.assertNotEqual(page.approved_uid, chreq.approved_uid)
+
+        # Cancel and return to draft
+        chreq.with_user(self.user2).action_cancel()
+        self.assertEqual(chreq.state, "cancelled")
+
+        chreq.with_user(self.user2).action_draft()
+        self.assertEqual(chreq.state, "draft")
 
         chreq.action_to_approve()
         self.assertEqual(chreq.state, "to approve")
-        self.assertNotEqual(page.content, chreq.content)
+        chreq.with_user(self.user2).action_approve()
-        self.assertNotEqual(page.approved_date, chreq.approved_date)
-        self.assertNotEqual(page.approved_uid, chreq.approved_uid)
-
-        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.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.action_approve()
         self.assertEqual(chreq.state, "approved")
         self.assertEqual(page.content, chreq.content)
         self.assertEqual(page.approved_date, chreq.approved_date)
         self.assertEqual(page.approved_uid, chreq.approved_uid)
 
     def test_get_approvers_guids(self):
-        """Get approver guids."""
+        """Test that approver groups are properly assigned."""
         page = self.page2
         self.assertTrue(len(page.approver_group_ids) > 0)
 
     def test_get_page_url(self):
-        """Test if page url exist."""
+        """Test that the page URL exists."""
         pages = self.env["document.page.history"].search([])
         page = pages[0]
         self.assertIsNotNone(page.page_url)
+
+    def test_compute_is_approval_required(self):
+        """Ensure approval rules are inherited correctly"""
+        self.assertTrue(self.page2.is_approval_required)
+        self.page2.parent_id.approval_required = False
+        self.page2.invalidate_model()
+        self.assertFalse(self.page2.is_approval_required)
+
+    def test_compute_approver_group_ids(self):
+        """Ensure approver groups are inherited correctly"""
+        self.assertIn(self.approver_gid, self.page2.approver_group_ids)
+        self.page2.parent_id.approver_gid = False
+        self.page2.invalidate_model()
+        self.assertFalse(self.page2.approver_group_ids)
+
+    def test_can_user_approve_this_page(self):
+        """Check different approval conditions"""
+        self.assertTrue(
+            self.page2.with_user(self.user2).can_user_approve_this_page(self.user2)
+        )
+
+        # Remove approval group from user2
+        self.user2.write({"groups_id": [(3, self.approver_gid.id)]})
+        self.assertFalse(
+            self.page2.with_user(self.user2).can_user_approve_this_page(self.user2)
+        )
+
+    def test_pending_approval_detection(self):
+        """Ensure the system detects pending approval changes"""
+        # Reset page2 by removing previous history
+        self.history_obj.search([("page_id", "=", self.page2.id)]).unlink()
+
+        self.page2.invalidate_model()
+        self.assertFalse(self.page2.has_changes_pending_approval)
+
+        # Create a new change request
+        self.history_obj.create(
+            {
+                "page_id": self.page2.id,
+                "state": "to approve",
+            }
+        )
+
+        self.page2.invalidate_model()
+        self.assertTrue(self.page2.has_changes_pending_approval)
+
+    def test_user_has_drafts(self):
+        """Ensure the system detects drafts correctly"""
+        self.page2.invalidate_model()
+        self.assertFalse(self.page2.user_has_drafts)
+
+        self.history_obj.create(
+            {
+                "page_id": self.page2.id,
+                "state": "draft",
+            }
+        )
+        self.page2.invalidate_model()
+        self.assertTrue(self.page2.user_has_drafts)
+
+    def test_action_draft_requires_cancellation(self):
+        """Ensure a change request must be cancelled before setting to draft"""
+        chreq = self.history_obj.create(
+            {
+                "page_id": self.page2.id,
+                "state": "to approve",
+            }
+        )
+        with self.assertRaises(UserError):
+            chreq.action_draft()
+
+    def test_action_to_approve_only_from_draft(self):
+        """Ensure only draft requests can be sent for approval"""
+        chreq = self.history_obj.create(
+            {
+                "page_id": self.page2.id,
+                "state": "approved",
+            }
+        )
+        with self.assertRaises(UserError):
+            chreq.action_to_approve()
+
+    def test_approval_permission_check(self):
+        """Ensure approval is restricted to approvers"""
+        chreq = self.history_obj.create(
+            {
+                "page_id": self.page2.id,
+                "state": "to approve",
+            }
+        )
+
+        with self.assertRaises(UserError):
+            chreq.with_user(self.env.ref("base.user_demo")).action_approve()
+
+        # Grant approval rights
+        chreq.with_user(self.user2).action_approve()
+        self.assertEqual(chreq.state, "approved")
+
+    def test_page_url_computation(self):
+        """Ensure page URLs are generated correctly"""
+        chreq = self.history_obj.create({"page_id": self.page2.id})
+        self.assertIn("web#db=", chreq.page_url)
+
+    def test_diff_computation(self):
+        """Ensure document diff is calculated properly"""
+        self.history_obj.create(
+            {
+                "page_id": self.page2.id,
+                "content": "Version 1",
+                "state": "approved",
+            }
+        )
+
+        chreq2 = self.history_obj.create(
+            {
+                "page_id": self.page2.id,
+                "content": "Version 2",
+            }
+        )
+        chreq2._compute_diff()
+        self.assertIsNotNone(chreq2.diff)

document_page_approval/views/document_page_approval.xml

@@ -90,10 +90,7 @@
             </field>
             <!-- Chatter -->
             <sheet position="after">
-                <div class="oe_chatter">
+                <chatter />
-                    <field name="message_follower_ids" widget="mail_followers" />
-                    <field name="message_ids" widget="mail_thread" />
-                </div>
             </sheet>
         </field>
     </record>
@@ -178,13 +175,13 @@
                 <field name="approved_date" />
             </field>
             <field name="history_ids" position="inside">
-                <tree>
+                <list>
                     <field name="id" />
                     <field name="approved_date" />
                     <field name="summary" />
                     <field name="create_uid" />
                     <field name="approved_uid" />
-                </tree>
+                </list>
             </field>
         </field>
     </record>
@@ -224,16 +221,16 @@
         <field name="model">document.page.history</field>
         <field name="inherit_id" ref="document_page.view_wiki_history_tree" />
         <field name="arch" type="xml">
-            <tree position="attributes">
+            <list position="attributes">
                 <attribute name="decoration-info">state=='draft'</attribute>
                 <attribute name="decoration-primary">state=='to approve'</attribute>
                 <attribute name="decoration-muted">state=='cancelled'</attribute>
-            </tree>
+            </list>
-            <tree position="inside">
+            <list position="inside">
                 <field name="state" />
                 <field name="approved_uid" />
                 <field name="approved_date" />
-            </tree>
+            </list>
         </field>
     </record>
     <!-- History Search view  -->
@@ -283,7 +280,7 @@
     <record model="ir.actions.act_window" id="action_change_requests">
         <field name="name">Change Requests</field>
         <field name="res_model">document.page.history</field>
-        <field name="view_mode">tree,form</field>
+        <field name="view_mode">list,form</field>
         <field name="context">{'search_default_draft': 1, 'search_default_pending': 1}
         </field>
     </record>