diff --git a/document_page/models/document_page_history.py b/document_page/models/document_page_history.py index 3bc2bda11f4..d52da93b901 100644 --- a/document_page/models/document_page_history.py +++ b/document_page/models/document_page_history.py @@ -54,4 +54,7 @@ def _get_diff(self, v1, v2): @api.depends("page_id") def _compute_display_name(self): for rec in self: - rec.display_name = (rec.id, f"{rec.page_id.name} #{rec.id}") + if isinstance(rec.id, api.NewId): + rec.display_name = rec.page_id.name or "" + else: + rec.display_name = f"{rec.page_id.name} #{rec.id}" diff --git a/document_page_approval/models/document_page.py b/document_page_approval/models/document_page.py index a2aa1117ce4..f86bee42586 100644 --- a/document_page_approval/models/document_page.py +++ b/document_page_approval/models/document_page.py @@ -88,6 +88,7 @@ def _compute_approver_group_ids(self): page.approver_group_ids = res @api.depends("is_approval_required", "approver_group_ids") + @api.depends_context("uid") def _compute_am_i_approver(self): """Check if the current user can approve changes to this page.""" for rec in self: @@ -111,6 +112,7 @@ def can_user_approve_this_page(self, user): # to approve, user must belong to any of the approver groups return len(user.group_ids & self.approver_group_ids) > 0 + @api.depends_context("uid") def _compute_has_changes_pending_approval(self): history = self.env["document.page.history"] for rec in self: @@ -119,6 +121,7 @@ def _compute_has_changes_pending_approval(self): ) rec.has_changes_pending_approval = changes > 0 + @api.depends_context("uid") def _compute_user_has_drafts(self): history = self.env["document.page.history"] for rec in self: diff --git a/document_page_approval/models/document_page_history.py b/document_page_approval/models/document_page_history.py index f8572e361ad..ef097ddcc8a 100644 --- a/document_page_approval/models/document_page_history.py +++ b/document_page_approval/models/document_page_history.py @@ -2,8 +2,9 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -from odoo import fields, models +from odoo import api, fields, models from odoo.exceptions import UserError +from odoo.tools.misc import clean_context class DocumentPageHistory(models.Model): @@ -83,7 +84,10 @@ def action_to_approve(self): [("group_ids", "in", guids), ("group_ids", "in", approver_gid.id)] ) rec.message_subscribe(partner_ids=users.mapped("partner_id").ids) - rec.message_post_with_source(template) + # pylint: disable=W8121 + rec.with_context( + clean_context(self.env.context) + ).message_post_with_source(template) else: # auto-approve if approval is not required rec.action_approve() @@ -149,6 +153,7 @@ def action_cancel_and_draft(self): self.action_cancel() self.action_draft() + @api.depends_context("uid") def _compute_am_i_owner(self): """Check if current user is the owner""" for rec in self: diff --git a/document_page_approval/tests/test_document_page_approval.py b/document_page_approval/tests/test_document_page_approval.py index d645e8c3042..cddae5fba21 100644 --- a/document_page_approval/tests/test_document_page_approval.py +++ b/document_page_approval/tests/test_document_page_approval.py @@ -1,5 +1,6 @@ from odoo.exceptions import UserError from odoo.tests import new_test_user +from odoo.tools import mute_logger from odoo.addons.base.tests.common import BaseCommon @@ -43,6 +44,13 @@ def setUpClass(cls): "type": "content", } ) + cls.user_normal = new_test_user( + cls.env, + login="test_normal", + groups="document_knowledge.group_document_user,document_page.group_document_editor", + name="Test Normal User", + email="normal@example.com", + ) # Ensure user2 has the approver group cls.approver_gid = cls.env.ref( @@ -73,6 +81,7 @@ def test_approval_required(self): self.assertTrue(page.has_changes_pending_approval) self.assertEqual(len(page.history_ids), 0) + @mute_logger("odoo.models.unlink") def test_change_request_approve(self): """Test that an approver can approve a change request.""" page = self.page2 @@ -92,6 +101,11 @@ def test_change_request_approve(self): self.assertEqual(chreq.state, "approved") self.assertEqual(chreq.content, page.content) + # Remove the linked mail.message and define a specific context to simulate that + # when accessing from the category smart button, there is no error when creating + # the history and sending the email + self.env["mail.message"].browse(page.parent_id.id).unlink() + page = page.with_context(default_parent_id=page.parent_id.id) # Create new change request page.write({"content": "
New content
"}) page.invalidate_model() # Recompute fields @@ -110,6 +124,7 @@ def test_change_request_auto_approve(self): page.write({"content": "New content
"}) self.assertEqual(page.content, "New content
") + @mute_logger("odoo.models.unlink") def test_change_request_from_scratch(self): """Test a full change request lifecycle from draft to approval.""" page = self.page2 @@ -184,6 +199,7 @@ def test_can_user_approve_this_page(self): self.page2.with_user(self.user2).can_user_approve_this_page(self.user2) ) + @mute_logger("odoo.models.unlink") def test_pending_approval_detection(self): """Ensure the system detects pending approval changes""" # Reset page2 by removing previous history @@ -282,3 +298,64 @@ def test_diff_computation(self): ) chreq2._compute_diff() self.assertIsNotNone(chreq2.diff) + + def test_cache_am_i_approver(self): + """Ensure 'am_i_approver' is computed per-user and not cached globally.""" + page = self.page2 + + # Normal user evaluates the field first (Calculates and caches: False) + self.assertFalse(page.with_user(self.user_normal).am_i_approver) + + # Approver user evaluates it next. + self.assertTrue(page.with_user(self.user2).am_i_approver) + + def test_cache_am_i_owner(self): + """Ensure 'am_i_owner' is computed per-user and not cached globally.""" + page = self.page2 + # Normal user creates the change request (they are the owner) + chreq = self.history_obj.with_user(self.user_normal).create( + { + "page_id": page.id, + "summary": "Cache Test", + "content": "User Normal Changes
", + } + ) + + # Approver user evaluates the field first (Calculates and caches: False) + self.assertFalse(chreq.with_user(self.user2).am_i_owner) + + # Normal user evaluates it next + self.assertTrue(chreq.with_user(self.user_normal).am_i_owner) + + def test_cache_user_has_drafts(self): + """Ensure 'user_has_drafts' is computed per-user and not cached globally.""" + page = self.page2 + # Normal user creates a draft change request + self.history_obj.with_user(self.user_normal).create( + { + "page_id": page.id, + "state": "draft", + } + ) + + # Approver evaluates the field first + self.assertFalse(page.with_user(self.user2).user_has_drafts) + + # Normal user evaluates it next + self.assertTrue(page.with_user(self.user_normal).user_has_drafts) + + def test_cache_has_changes_pending_approval(self): + """Ensure 'has_changes_pending_approval' is computed per-user and not cached.""" + page = self.page2 + # Create a pending approval change request using the Manager + # so it's NOT owned by the normal user. + chreq = self.history_obj.with_user(self.manager_user).create( + {"page_id": page.id, "content": "Manager Change
"} + ) + chreq.with_user(self.manager_user).action_to_approve() + + # Normal user evaluates the field. + self.assertFalse(page.with_user(self.user_normal).has_changes_pending_approval) + + # Approver user evaluates it. + self.assertTrue(page.with_user(self.user2).has_changes_pending_approval)