move fire_policy_webhooks in engine

Signed-off-by: tdruez <tdruez@aboutcode.org>

Author: tdruez

policy/engine.py

@@ -8,10 +8,30 @@
 
 from django.utils import timezone
 
+from notification.models import fire_webhooks
 from policy.rules import RULE_REGISTRY
 from product_portfolio.models import ProductPolicyViolation
 
 
+def fire_policy_webhooks(product, new_violations, resolved_count):
+    """Fire policy webhooks for newly detected or resolved violations."""
+    if new_violations:
+        lines = [
+            f"- {violation.rule_label}: {violation.violation_count} violation(s)"
+            for violation in new_violations
+        ]
+        payload = {
+            "text": (f"[DejaCode] Policy violations detected for {product}\n" + "\n".join(lines))
+        }
+        fire_webhooks("policy.violation_detected", instance=product, payload_override=payload)
+
+    if resolved_count:
+        payload = {
+            "text": (f"[DejaCode] {resolved_count} policy violation(s) resolved for {product}")
+        }
+        fire_webhooks("policy.violation_resolved", instance=product, payload_override=payload)
+
+
 def get_effective_config(rule_type, dataspace):
     """
     Resolve threshold, parameters, and is_active for a rule type in a given dataspace.
@@ -95,4 +115,5 @@ def evaluate_rules(product):
             new_violations.append(violation)
         resolved_count += resolved
 
+    fire_policy_webhooks(product, new_violations, resolved_count)
     return new_violations, resolved_count

policy/tasks.py

@@ -13,31 +13,11 @@
 from django_rq import job
 
 from dje.models import get_unsecured_manager
-from notification.models import fire_webhooks
 from policy.engine import evaluate_rules
 
 logger = logging.getLogger(__name__)
 
 
-def fire_policy_webhooks(product, new_violations, resolved_count):
-    """Fire policy webhooks for newly detected or resolved violations."""
-    if new_violations:
-        lines = [
-            f"- {violation.rule_label}: {violation.violation_count} violation(s)"
-            for violation in new_violations
-        ]
-        payload = {
-            "text": (f"[DejaCode] Policy violations detected for {product}\n" + "\n".join(lines))
-        }
-        fire_webhooks("policy.violation_detected", instance=product, payload_override=payload)
-
-    if resolved_count:
-        payload = {
-            "text": (f"[DejaCode] {resolved_count} policy violation(s) resolved for {product}")
-        }
-        fire_webhooks("policy.violation_resolved", instance=product, payload_override=payload)
-
-
 @job
 def evaluate_product_rules_task(product_uuid):
     """Evaluate all active policy rules for the given product and fire webhooks on changes."""
@@ -55,7 +35,6 @@ def evaluate_product_rules_task(product_uuid):
         f"Policy rules evaluated for {product}: "
         f"{len(new_violations)} new violation(s), {resolved_count} resolved."
     )
-    fire_policy_webhooks(product, new_violations, resolved_count)
 
 
 @job
@@ -82,6 +61,5 @@ def evaluate_all_products_rules_task(include_locked=False, product_uuids=None):
             f"Policy rules evaluated for {product}: "
             f"{len(new_violations)} new violation(s), {resolved_count} resolved."
         )
-        fire_policy_webhooks(product, new_violations, resolved_count)
 
     logger.info(f"Policy rule evaluation complete for {count} product(s).")

policy/tests/test_tasks.py

@@ -13,9 +13,9 @@
 from django.test import TestCase
 
 from dje.models import Dataspace
+from policy.engine import fire_policy_webhooks
 from policy.tasks import evaluate_all_products_rules_task
 from policy.tasks import evaluate_product_rules_task
-from policy.tasks import fire_policy_webhooks
 from product_portfolio.tests import make_product
 from product_portfolio.tests import make_product_status
 
@@ -25,7 +25,7 @@ def setUp(self):
         self.dataspace = Dataspace.objects.create(name="nexB")
         self.product = make_product(self.dataspace)
 
-    @patch("policy.tasks.fire_webhooks")
+    @patch("policy.engine.fire_webhooks")
     def test_fire_policy_webhooks_dispatches_violation_detected(self, mock_fire):
         violation = MagicMock()
         violation.rule_label = "Usage Policy Error"
@@ -37,15 +37,15 @@ def test_fire_policy_webhooks_dispatches_violation_detected(self, mock_fire):
         self.assertIn("Policy violations detected", kwargs["payload_override"]["text"])
         self.assertIn("Usage Policy Error", kwargs["payload_override"]["text"])
 
-    @patch("policy.tasks.fire_webhooks")
+    @patch("policy.engine.fire_webhooks")
     def test_fire_policy_webhooks_dispatches_violation_resolved(self, mock_fire):
         fire_policy_webhooks(self.product, new_violations=[], resolved_count=2)
         mock_fire.assert_called_once()
         event_name, kwargs = mock_fire.call_args[0][0], mock_fire.call_args[1]
         self.assertEqual("policy.violation_resolved", event_name)
         self.assertIn("2 policy violation(s) resolved", kwargs["payload_override"]["text"])
 
-    @patch("policy.tasks.fire_webhooks")
+    @patch("policy.engine.fire_webhooks")
     def test_fire_policy_webhooks_dispatches_both_events(self, mock_fire):
         violation = MagicMock()
         violation.rule_label = "License Coverage Gap"
@@ -56,7 +56,7 @@ def test_fire_policy_webhooks_dispatches_both_events(self, mock_fire):
         self.assertIn("policy.violation_detected", events_fired)
         self.assertIn("policy.violation_resolved", events_fired)
 
-    @patch("policy.tasks.fire_webhooks")
+    @patch("policy.engine.fire_webhooks")
     def test_fire_policy_webhooks_silent_when_no_changes(self, mock_fire):
         fire_policy_webhooks(self.product, new_violations=[], resolved_count=0)
         mock_fire.assert_not_called()
@@ -67,31 +67,26 @@ def setUp(self):
         self.dataspace = Dataspace.objects.create(name="nexB")
         self.product = make_product(self.dataspace)
 
-    @patch("policy.tasks.fire_policy_webhooks")
     @patch("policy.tasks.evaluate_rules")
-    def test_evaluate_product_rules_task_runs_evaluation(self, mock_evaluate, mock_fire):
+    def test_evaluate_product_rules_task_runs_evaluation(self, mock_evaluate):
         mock_evaluate.return_value = ([], 0)
         evaluate_product_rules_task(product_uuid=self.product.uuid)
         mock_evaluate.assert_called_once_with(self.product)
-        mock_fire.assert_called_once_with(self.product, [], 0)
 
-    @patch("policy.tasks.fire_policy_webhooks")
     @patch("policy.tasks.evaluate_rules")
-    def test_evaluate_product_rules_task_unknown_uuid_logs_error(self, mock_evaluate, mock_fire):
+    def test_evaluate_product_rules_task_unknown_uuid_logs_error(self, mock_evaluate):
         with self.assertLogs("policy.tasks", level="ERROR") as captured:
             evaluate_product_rules_task(product_uuid=uuid.uuid4())
         mock_evaluate.assert_not_called()
-        mock_fire.assert_not_called()
         self.assertTrue(any("not found" in line for line in captured.output))
 
 
 class EvaluateAllProductsRulesTaskTestCase(TestCase):
     def setUp(self):
         self.dataspace = Dataspace.objects.create(name="nexB")
 
-    @patch("policy.tasks.fire_policy_webhooks")
     @patch("policy.tasks.evaluate_rules")
-    def test_evaluate_all_products_excludes_locked_by_default(self, mock_evaluate, mock_fire):
+    def test_evaluate_all_products_excludes_locked_by_default(self, mock_evaluate):
         # Regression: previously called .exclude_locked() on DataspacedQuerySet which lacks that
         # method. Now uses .exclude(configuration_status__is_locked=True) inline.
         mock_evaluate.return_value = ([], 0)
@@ -106,9 +101,8 @@ def test_evaluate_all_products_excludes_locked_by_default(self, mock_evaluate, m
         self.assertIn(active_product, evaluated_products)
         self.assertNotIn(locked_product, evaluated_products)
 
-    @patch("policy.tasks.fire_policy_webhooks")
     @patch("policy.tasks.evaluate_rules")
-    def test_evaluate_all_products_includes_locked_when_requested(self, mock_evaluate, mock_fire):
+    def test_evaluate_all_products_includes_locked_when_requested(self, mock_evaluate):
         mock_evaluate.return_value = ([], 0)
         locked_status = make_product_status(self.dataspace, is_locked=True)
         locked_product = make_product(self.dataspace, configuration_status=locked_status)
@@ -119,9 +113,8 @@ def test_evaluate_all_products_includes_locked_when_requested(self, mock_evaluat
         evaluated_products = [c[0][0] for c in mock_evaluate.call_args_list]
         self.assertIn(locked_product, evaluated_products)
 
-    @patch("policy.tasks.fire_policy_webhooks")
     @patch("policy.tasks.evaluate_rules")
-    def test_evaluate_all_products_filters_by_uuids(self, mock_evaluate, mock_fire):
+    def test_evaluate_all_products_filters_by_uuids(self, mock_evaluate):
         mock_evaluate.return_value = ([], 0)
         product_a = make_product(self.dataspace)
         product_b = make_product(self.dataspace)
@@ -133,11 +126,8 @@ def test_evaluate_all_products_filters_by_uuids(self, mock_evaluate, mock_fire):
         self.assertIn(product_a, evaluated_products)
         self.assertNotIn(product_b, evaluated_products)
 
-    @patch("policy.tasks.fire_policy_webhooks")
     @patch("policy.tasks.evaluate_rules")
-    def test_evaluate_all_products_uuid_filter_ignores_locked_exclusion(
-        self, mock_evaluate, mock_fire
-    ):
+    def test_evaluate_all_products_uuid_filter_ignores_locked_exclusion(self, mock_evaluate):
         mock_evaluate.return_value = ([], 0)
         locked_status = make_product_status(self.dataspace, is_locked=True)
         locked_product = make_product(self.dataspace, configuration_status=locked_status)