fix: comment config can be bool (#748)

codecov · Oct 1, 2024 · 9ad07e3 · 9ad07e3
1 parent e43cf58
commit 9ad07e3
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 18 deletions.
diff --git a/services/bundle_analysis/notify/contexts/__init__.py b/services/bundle_analysis/notify/contexts/__init__.py
@@ -3,10 +3,7 @@
 from typing import Generic, Literal, Self, TypeVar
 
 import sentry_sdk
-from shared.bundle_analysis import (
-    BundleAnalysisReport,
-    BundleAnalysisReportLoader,
-)
+from shared.bundle_analysis import BundleAnalysisReport, BundleAnalysisReportLoader
 from shared.torngit.base import TorngitBaseAdapter
 from shared.validation.types import BundleThreshold
 from shared.yaml import UserYaml
@@ -20,9 +17,7 @@
     NotificationType,
     NotificationUserConfig,
 )
-from services.repository import (
-    get_repo_provider_service,
-)
+from services.repository import get_repo_provider_service
 from services.storage import get_storage_client
 
 T = TypeVar("T")
@@ -192,16 +187,18 @@ def load_user_config(self) -> "NotificationContextBuilder":
 
         This allows all notifiers to access configuration for any notifier and already have the defaults
         """
-        required_changes: bool | Literal["bundle_increase"] = (
-            self.current_yaml.read_yaml_field(
-                "comment", "require_bundle_changes", _else=False
+        comment_config: bool | dict = self.current_yaml.read_yaml_field("comment")
+        if not comment_config:
+            required_changes = False
+            required_changes_threshold = BundleThreshold("absolute", 0)
+        else:
+            required_changes: bool | Literal["bundle_increase"] = comment_config.get(
+                "require_bundle_changes", False
+            )
+            required_changes_threshold: int | float = comment_config.get(
+                "bundle_change_threshold",
+                BundleThreshold("absolute", 0),
             )
-        )
-        required_changes_threshold: int | float = self.current_yaml.read_yaml_field(
-            "comment",
-            "bundle_change_threshold",
-            _else=BundleThreshold("absolute", 0),
-        )
         warning_threshold: int | float = self.current_yaml.read_yaml_field(
             "bundle_analysis",
             "warning_threshold",

diff --git a/services/bundle_analysis/notify/contexts/tests/test_contexts.py b/services/bundle_analysis/notify/contexts/tests/test_contexts.py
@@ -1,6 +1,7 @@
 from unittest.mock import MagicMock
 
 import pytest
+from shared.validation.types import BundleThreshold
 from shared.yaml import UserYaml
 
 from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME
@@ -14,6 +15,7 @@
     NotificationContextBuilder,
     NotificationContextBuildError,
 )
+from services.bundle_analysis.notify.types import NotificationUserConfig
 
 
 class TestBaseBundleAnalysisNotificationContextBuild:
@@ -67,7 +69,40 @@ def test_load_bundle_analysis_report_no_report(self, dbsession, mock_storage):
             builder.load_bundle_analysis_report()
         assert exp.value.failed_step == "load_bundle_analysis_report"
 
-    def test_build_context(self, dbsession, mock_storage, mocker):
+    @pytest.mark.parametrize(
+        "config, expected_user_config",
+        [
+            pytest.param(
+                {
+                    "comment": {
+                        "layout": "reach,diff,flags,tree,reach",
+                        "behavior": "default",
+                        "show_carryforward_flags": False,
+                    }
+                },
+                NotificationUserConfig(
+                    required_changes=False,
+                    warning_threshold=BundleThreshold("percentage", 5.0),
+                    status_level="informational",
+                    required_changes_threshold=BundleThreshold("absolute", 0),
+                ),
+                id="default_site_config",
+            ),
+            pytest.param(
+                {"comment": False},
+                NotificationUserConfig(
+                    required_changes=False,
+                    warning_threshold=BundleThreshold("percentage", 5.0),
+                    status_level="informational",
+                    required_changes_threshold=BundleThreshold("absolute", 0),
+                ),
+                id="comment_is_bool",
+            ),
+        ],
+    )
+    def test_build_context(
+        self, dbsession, mock_storage, mocker, config, expected_user_config
+    ):
         head_commit, base_commit = get_commit_pair(dbsession)
         head_commit_report, _ = get_report_pair(dbsession, (head_commit, base_commit))
         save_mock_bundle_analysis_report(
@@ -77,11 +112,14 @@ def test_build_context(self, dbsession, mock_storage, mocker):
             sample_report_number=1,
         )
         builder = NotificationContextBuilder().initialize(
-            head_commit, UserYaml.from_dict({}), GITHUB_APP_INSTALLATION_DEFAULT_NAME
+            head_commit,
+            UserYaml.from_dict(config),
+            GITHUB_APP_INSTALLATION_DEFAULT_NAME,
         )
         context = builder.build_context().get_result()
         assert context.commit_report == head_commit_report
         assert context.bundle_analysis_report.session_count() == 19
+        assert context.user_config == expected_user_config
         assert [
             bundle_report.name
             for bundle_report in context.bundle_analysis_report.bundle_reports()