[FIX] Overly long function verify_endpoint_health

hookwise/tasks.py

@@ -5,7 +5,7 @@
 import re
 import time
 from datetime import datetime, timedelta, timezone
-from typing import Any, Dict, Optional, cast
+from typing import Any, Dict, Optional, Set, Tuple, cast
 
 from celery import Celery, Task
 from prometheus_client import Counter, Histogram
@@ -129,54 +129,70 @@ def cleanup_logs() -> None:
     logger.info(f"Cleaned up {deleted} log entries older than {retention_days} days.")
 
 
+def _get_health_check_metadata() -> Optional[Dict[str, Any]]:
+    """Fetch and prepare metadata for health check."""
+    boards = cw_client.get_boards()
+    if not boards:
+        return None
+
+    priorities = cw_client.get_priorities()
+    return {
+        "board_map": {b["name"]: b["id"] for b in boards},
+        "priority_names": {p["name"] for p in priorities},
+    }
+
+
+def _validate_single_config_health(
+    config: WebhookConfig, board_map: Dict[str, int], priority_names: Set[str], status_cache: Dict[int, Any]
+) -> Tuple[str, str]:
+    """Validate a single configuration and return status and message."""
+    errors = []
+
+    # 1. Check Board
+    if config.board:
+        if config.board not in board_map:
+            errors.append(f"Board '{config.board}' not found")
+        else:
+            # 2. Check Status
+            if config.status:
+                bid = board_map[config.board]
+                if bid not in status_cache:
+                    statuses = cw_client.get_board_statuses(bid)
+                    status_cache[bid] = {s["name"] for s in statuses}
+
+                if config.status not in status_cache[bid]:
+                    errors.append(f"Status '{config.status}' not found")
+
+    # 3. Check Priority
+    if config.priority and config.priority not in priority_names:
+        errors.append(f"Priority '{config.priority}' not found")
+
+    # Determine Status
+    if errors:
+        return "ERROR", " | ".join(errors)
+    return "OK", "Configuration validated"
+
+
 @celery.task(name="hookwise.verify_endpoint_health")  # type: ignore[untyped-decorator]
 def verify_endpoint_health() -> None:
     """Validate endpoint configurations against ConnectWise."""
     try:
-        # Fetch global metadata
-        boards = cw_client.get_boards()
-        if not boards:
+        metadata = _get_health_check_metadata()
+        if not metadata:
             logger.warning("Skipping health check: Unable to fetch boards from CW.")
             return
 
-        board_map = {b["name"]: b["id"] for b in boards}
-
-        priorities = cw_client.get_priorities()
-        priority_names = {p["name"] for p in priorities}
-
+        board_map = metadata["board_map"]
+        priority_names = metadata["priority_names"]
         status_cache: Dict[int, Any] = {}
 
         configs = WebhookConfig.query.filter_by(is_enabled=True).all()
         updates = 0
 
         for config in configs:
-            errors = []
-
-            # 1. Check Board
-            if config.board:
-                if config.board not in board_map:
-                    errors.append(f"Board '{config.board}' not found")
-                else:
-                    # 2. Check Status
-                    if config.status:
-                        bid = board_map[config.board]
-                        if bid not in status_cache:
-                            statuses = cw_client.get_board_statuses(bid)
-                            status_cache[bid] = {s["name"] for s in statuses}
-
-                        if config.status not in status_cache[bid]:
-                            errors.append(f"Status '{config.status}' not found")
-
-            # 3. Check Priority
-            if config.priority and config.priority not in priority_names:
-                errors.append(f"Priority '{config.priority}' not found")
-
-            # Determine Status
-            new_status = "OK"
-            new_msg = "Configuration validated"
-            if errors:
-                new_status = "ERROR"
-                new_msg = " | ".join(errors)
+            new_status, new_msg = _validate_single_config_health(
+                config, board_map, priority_names, status_cache
+            )
 
             # Update if changed
             if config.config_health_status != new_status or config.config_health_message != new_msg:

tests/test_health_task.py

@@ -0,0 +1,114 @@
+import pytest
+import os
+from unittest.mock import patch, MagicMock
+from hookwise import create_app
+from hookwise.extensions import db
+from hookwise.models import WebhookConfig
+from hookwise.tasks import verify_endpoint_health
+
+@pytest.fixture
+def app():
+    os.environ["GUI_PASSWORD"] = "testpass"
+    _app = create_app()
+    _app.config["TESTING"] = True
+    _app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite:///test_health.db"
+    with _app.app_context():
+        db.create_all()
+        # Mock hookwise.tasks._app to use our test app
+        with patch("hookwise.tasks._app", _app):
+            yield _app
+        db.session.remove()
+        db.drop_all()
+    if os.path.exists("test_health.db"):
+        try:
+            os.remove("test_health.db")
+        except:
+            pass
+
+@patch("hookwise.tasks.cw_client")
+def test_verify_endpoint_health_success(mock_cw, app):
+    """Test that valid configurations are marked as OK."""
+    mock_cw.get_boards.return_value = [{"name": "Test Board", "id": 1}]
+    mock_cw.get_priorities.return_value = [{"name": "P1"}]
+    mock_cw.get_board_statuses.return_value = [{"name": "New"}]
+
+    with app.app_context():
+        config = WebhookConfig(
+            name="Valid Config",
+            board="Test Board",
+            status="New",
+            priority="P1",
+            is_enabled=True
+        )
+        db.session.add(config)
+        db.session.commit()
+
+        verify_endpoint_health()
+
+        db.session.refresh(config)
+        assert config.config_health_status == "OK"
+        assert config.config_health_message == "Configuration validated"
+
+@patch("hookwise.tasks.cw_client")
+def test_verify_endpoint_health_errors(mock_cw, app):
+    """Test that invalid configurations are marked with ERROR and messages."""
+    mock_cw.get_boards.return_value = [{"name": "Valid Board", "id": 1}]
+    mock_cw.get_priorities.return_value = [{"name": "Valid Priority"}]
+    mock_cw.get_board_statuses.return_value = [{"name": "Valid Status"}]
+
+    with app.app_context():
+        # 1. Invalid Board
+        config_bad_board = WebhookConfig(
+            name="Bad Board",
+            board="Invalid Board",
+            is_enabled=True
+        )
+
+        # 2. Invalid Status
+        config_bad_status = WebhookConfig(
+            name="Bad Status",
+            board="Valid Board",
+            status="Invalid Status",
+            is_enabled=True
+        )
+
+        # 3. Invalid Priority
+        config_bad_priority = WebhookConfig(
+            name="Bad Priority",
+            priority="Invalid Priority",
+            is_enabled=True
+        )
+
+        db.session.add_all([config_bad_board, config_bad_status, config_bad_priority])
+        db.session.commit()
+
+        verify_endpoint_health()
+
+        db.session.refresh(config_bad_board)
+        db.session.refresh(config_bad_status)
+        db.session.refresh(config_bad_priority)
+
+        assert config_bad_board.config_health_status == "ERROR"
+        assert "Board 'Invalid Board' not found" in config_bad_board.config_health_message
+
+        assert config_bad_status.config_health_status == "ERROR"
+        assert "Status 'Invalid Status' not found" in config_bad_status.config_health_message
+
+        assert config_bad_priority.config_health_status == "ERROR"
+        assert "Priority 'Invalid Priority' not found" in config_bad_priority.config_health_message
+
+@patch("hookwise.tasks.cw_client")
+def test_verify_endpoint_health_cw_failure(mock_cw, app):
+    """Test that if CW is unreachable, it logs a warning and returns."""
+    mock_cw.get_boards.return_value = []
+
+    with app.app_context():
+        config = WebhookConfig(name="Test", is_enabled=True)
+        db.session.add(config)
+        db.session.commit()
+
+        # Should return early without updating anything
+        verify_endpoint_health()
+
+        db.session.refresh(config)
+        assert config.config_health_status == "OK" # Default value