[FIX] improve _compute_all_closed method of helpdesk.ticket

Checking a boolean field is more reliable than checking arbitrary names that can be changed by the user
OCA · Oct 21, 2024 · 459c3e8 · 459c3e8
1 parent 42c7cf9
commit 459c3e8
Show file tree

Hide file tree

Showing 6 changed files with 113 additions and 44 deletions.
diff --git a/helpdesk_mgmt_fieldservice/__manifest__.py b/helpdesk_mgmt_fieldservice/__manifest__.py
@@ -5,7 +5,7 @@
     "name": "Helpdesk Mgmt Fieldservice",
     "summary": """
         Create service orders from a ticket""",
-    "version": "14.0.1.1.2",
+    "version": "14.0.1.2.0",
     "license": "AGPL-3",
     "author": "Open Source Integrators, "
     "Escodoo, "

diff --git a/helpdesk_mgmt_fieldservice/migrations/14.0.1.2.0/post-migrate.py b/helpdesk_mgmt_fieldservice/migrations/14.0.1.2.0/post-migrate.py
@@ -0,0 +1,7 @@
+from odoo import SUPERUSER_ID, api
+
+
+def migrate(cr, version):
+    """Recalculate all_orders_closed after fix"""
+    env = api.Environment(cr, SUPERUSER_ID, {"active_test": False})
+    env["helpdesk.ticket"].search([])._compute_all_closed()
diff --git a/helpdesk_mgmt_fieldservice/models/fsm_order.py b/helpdesk_mgmt_fieldservice/models/fsm_order.py
@@ -13,16 +13,9 @@ class FSMOrder(models.Model):
     def action_complete(self):
         res = super().action_complete()
         if self.ticket_id:
-            open_fsm_orders_count = self.env["fsm.order"].search_count(
-                [
-                    ("ticket_id", "=", self.ticket_id.id),
-                    ("stage_id.is_closed", "=", False),
-                ]
-            )
-
             if self.ticket_id.stage_id.closed:
                 return res
-            elif open_fsm_orders_count == 0:
+            elif self.ticket_id.all_orders_closed:
                 view_id = self.env.ref(
                     "helpdesk_mgmt_fieldservice.fsm_order_close_wizard_view_form"
                 ).id

diff --git a/helpdesk_mgmt_fieldservice/models/helpdesk_ticket.py b/helpdesk_mgmt_fieldservice/models/helpdesk_ticket.py
@@ -19,21 +19,14 @@ class HelpdeskTicket(models.Model):
 
     @api.constrains("stage_id")
     def _validate_stage_fields(self):
-        for rec in self:
-            stage = rec.stage_id
-            if stage.closed:
-                if rec.fsm_order_ids:
-                    closed_orders = rec.fsm_order_ids.filtered(
-                        lambda x: x.stage_id.is_closed
+        for rec in self.filtered("stage_id.closed"):
+            if rec.fsm_order_ids and not rec.all_orders_closed:
+                raise ValidationError(
+                    _(
+                        "Please complete all service orders "
+                        "related to this ticket to close it."
                     )
-                    if len(closed_orders.ids) != len(rec.fsm_order_ids):
-
-                        raise ValidationError(
-                            _(
-                                "Please complete all service orders "
-                                "related to this ticket to close it."
-                            )
-                        )
+                )
 
     def _location_contact_fill(self, loc):
         """loc is a boolean that lets us know if this is coming from the
@@ -82,13 +75,12 @@ def action_create_order(self):
         action["views"] = [(res and res.id or False, "form")]
         return action
 
-    @api.depends("fsm_order_ids", "stage_id", "fsm_order_ids.stage_id")
+    @api.depends("fsm_order_ids.stage_id.is_closed")
     def _compute_all_closed(self):
         for ticket in self:
-            ticket.all_orders_closed = True
             if ticket.fsm_order_ids:
-                for order in ticket.fsm_order_ids:
-                    if order.stage_id.name not in ["Closed", "Cancelled"]:
-                        ticket.all_orders_closed = False
+                ticket.all_orders_closed = all(
+                    order.stage_id.is_closed for order in ticket.fsm_order_ids
+                )
             else:
                 ticket.all_orders_closed = False
diff --git a/helpdesk_mgmt_fieldservice/static/description/index.html b/helpdesk_mgmt_fieldservice/static/description/index.html
@@ -1,4 +1,3 @@
-<?xml version="1.0" encoding="utf-8"?>
 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
 <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
 <head>
@@ -9,10 +8,11 @@
 
 /*
 :Author: David Goodger ([email protected])
-:Id: $Id: html4css1.css 8954 2022-01-20 10:10:25Z milde $
+:Id: $Id: html4css1.css 9511 2024-01-13 09:50:07Z milde $
 :Copyright: This stylesheet has been placed in the public domain.
 
 Default cascading style sheet for the HTML output of Docutils.
+Despite the name, some widely supported CSS2 features are used.
 
 See https://docutils.sourceforge.io/docs/howto/html-stylesheets.html for how to
 customize this style sheet.
@@ -275,7 +275,7 @@
   margin-left: 2em ;
   margin-right: 2em }
 
-pre.code .ln { color: grey; } /* line numbers */
+pre.code .ln { color: gray; } /* line numbers */
 pre.code, code { background-color: #eeeeee }
 pre.code .comment, code .comment { color: #5C6576 }
 pre.code .keyword, code .keyword { color: #3B0D06; font-weight: bold }
@@ -301,7 +301,7 @@
 span.pre {
   white-space: pre }
 
-span.problematic {
+span.problematic, pre.problematic {
   color: red }
 
 span.section-subtitle {
@@ -448,7 +448,9 @@ <h2><a class="toc-backref" href="#toc-entry-9">Contributors</a></h2>
 <div class="section" id="maintainers">
 <h2><a class="toc-backref" href="#toc-entry-10">Maintainers</a></h2>
 <p>This module is maintained by the OCA.</p>
-<a class="reference external image-reference" href="https://odoo-community.org"><img alt="Odoo Community Association" src="https://odoo-community.org/logo.png" /></a>
+<a class="reference external image-reference" href="https://odoo-community.org">
+<img alt="Odoo Community Association" src="https://odoo-community.org/logo.png" />
+</a>
 <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>

diff --git a/helpdesk_mgmt_fieldservice/tests/test_helpdesk_ticket_fsm_order.py b/helpdesk_mgmt_fieldservice/tests/test_helpdesk_ticket_fsm_order.py
@@ -8,30 +8,33 @@ class TestHelpdeskTicketFSMOrder(SavepointCase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
-        cls.helpdesk_ticket = cls.env["helpdesk.ticket"]
-        cls.fsm_order = cls.env["fsm.order"]
+        cls.HelpdeskTicket = cls.env["helpdesk.ticket"]
+        cls.FsmOrder = cls.env["fsm.order"]
+        cls.FsmStage = cls.env["fsm.stage"]
         cls.partner = cls.env["res.partner"].create({"name": "Partner 1"})
         cls.user_demo = cls.env.ref("base.user_demo")
-        cls.helpdesk_ticket_team = cls.env["helpdesk.ticket.team"]
+        cls.HelpdeskTicketTeam = cls.env["helpdesk.ticket.team"]
         cls.fsm_team = cls.env["fsm.team"].create({"name": "FSM Team"})
-        cls.mail_alias = cls.env["mail.alias"]
+        cls.fsm_stage_new = cls.env.ref("fieldservice.fsm_stage_new")
+        cls.fsm_stage_cancelled = cls.env.ref("fieldservice.fsm_stage_cancelled")
+        cls.MailAlias = cls.env["mail.alias"]
         cls.stage_closed = cls.env.ref("helpdesk_mgmt.helpdesk_ticket_stage_done")
         cls.stage_completed = cls.env.ref("fieldservice.fsm_stage_completed")
         cls.test_location = cls.env.ref("fieldservice.test_location")
         cls.partner.service_location_id = cls.test_location
-        cls.mail_alias_id = cls.mail_alias.create(
+        cls.mail_alias_id = cls.MailAlias.create(
             {
                 "alias_name": "Test Mail Alias",
                 "alias_model_id": cls.env["ir.model"]
                 .search([("model", "=", "helpdesk.ticket")])
                 .id,
             }
         )
-        cls.team_id = cls.helpdesk_ticket_team.create(
+        cls.team_id = cls.HelpdeskTicketTeam.create(
             {"name": "Team 1", "alias_id": cls.mail_alias_id.id}
         )
 
-        cls.ticket_1 = cls.helpdesk_ticket.create(
+        cls.ticket_1 = cls.HelpdeskTicket.create(
             {
                 "name": "Test 1",
                 "description": "Ticket test",
@@ -40,7 +43,7 @@ def setUpClass(cls):
                 "fsm_location_id": cls.test_location.id,
             }
         )
-        cls.ticket_2 = cls.helpdesk_ticket.create(
+        cls.ticket_2 = cls.HelpdeskTicket.create(
             {
                 "name": "Test 2",
                 "description": "Ticket test",
@@ -49,13 +52,21 @@ def setUpClass(cls):
                 "fsm_location_id": cls.test_location.id,
             }
         )
-        cls.fsm_order_no_ticket = cls.fsm_order.create(
+        cls.fsm_order_no_ticket = cls.FsmOrder.create(
             {
                 "name": "No ticket order",
                 "location_id": cls.test_location.id,
                 "team_id": cls.fsm_team.id,
             }
         )
+        cls.fsm_stage_closed = cls.FsmStage.create(
+            {
+                "name": "Custom Closing Stage",
+                "stage_type": "order",
+                "is_closed": True,
+                "sequence": 200,
+            }
+        )
 
     def _create_fsm_orders(self, fsm_order_obj):
         f = Form(fsm_order_obj)
@@ -68,7 +79,7 @@ def test_helpdesk_ticket_fsm_order(self):
         """
         # checking action_create_order on fsm.order
         action_create_order = self.ticket_1.action_create_order()
-        fsm_order_obj = self.fsm_order.with_context(**action_create_order["context"])
+        fsm_order_obj = self.FsmOrder.with_context(**action_create_order["context"])
         fsm_orders = [self._create_fsm_orders(fsm_order_obj) for _ in range(5)]
         self.assertRecordValues(
             fsm_orders,
@@ -130,10 +141,74 @@ def test_helpdesk_ticket_fsm_order(self):
         f.stage_id = self.stage_closed
         close_wizard_form = f.save()
         close_wizard_form.action_close_ticket()
-        self.assertFalse(self.ticket_1.all_orders_closed)
+        self.assertTrue(self.ticket_1.all_orders_closed)
         self.assertEqual(self.ticket_1.stage_id.name, self.stage_closed.name)
         self.assertEqual(self.ticket_1.resolution, "Just another resolution")
         # check action_complete on fsm.order no ticket
         self.fsm_order_no_ticket.action_complete()
         self.assertEqual(self.fsm_order_no_ticket.stage_id, self.stage_completed)
         self.assertFalse(self.fsm_order_no_ticket.is_button)
+
+    def test_all_orders_closed(self):
+        """
+        One of the things this test is for is avoiding hardcoding the name
+        of the fsm.stage in the compute method of all_orders_closed
+        as was previously done
+        """
+
+        self.assertFalse(self.ticket_1.fsm_order_ids)
+        self.assertFalse(
+            self.ticket_1.all_orders_closed,
+            "Helpdesk Ticket: with no linked FSM Order, "
+            "all_orders_closed should be False",
+        )
+
+        action_create_order = self.ticket_1.action_create_order()
+        fsm_order_obj = self.FsmOrder.with_context(**action_create_order["context"])
+        fsm_orders = [self._create_fsm_orders(fsm_order_obj) for _ in range(3)]
+
+        self.assertFalse(
+            any(order.stage_id.is_closed for order in fsm_orders),
+            "FSM Orders should all be open by default",
+        )
+
+        self.assertFalse(
+            self.ticket_1.all_orders_closed,
+            "Helpdesk Ticket: with multiple orders open, "
+            "all_orders_closed should be False",
+        )
+
+        fsm_orders[0].stage_id = self.fsm_stage_closed
+        self.assertFalse(
+            self.ticket_1.all_orders_closed,
+            "Helpdesk Ticket: only one order closed, "
+            "all_orders_closed should still be False",
+        )
+
+        fsm_orders[1].ticket_id = False
+        self.assertFalse(
+            self.ticket_1.all_orders_closed,
+            "Helpdesk Ticket: one order closed, one open, one unlinked, "
+            "all_orders_closed should still be False",
+        )
+
+        fsm_orders[2].stage_id = self.fsm_stage_cancelled
+        self.assertTrue(
+            self.ticket_1.all_orders_closed,
+            "Helpdesk Ticket: one order closed, one cancelled, one unlinked, "
+            "all_orders_closed should be True",
+        )
+
+        fsm_orders[1].ticket_id = self.ticket_1.id
+        self.assertFalse(
+            self.ticket_1.all_orders_closed,
+            "Helpdesk Ticket: relinking previous order "
+            "all_orders_closed should be False",
+        )
+
+        fsm_orders[1].stage_id.is_closed = True
+        self.assertTrue(
+            self.ticket_1.all_orders_closed,
+            "Helpdesk Ticket: changed 'is_closed' attribute on order's state"
+            "all_orders_closed should be True",
+        )