modify core logic for efficiency and add tests for incident prs

Author: Kamlesh72

backend/analytics_server/mhq/service/incidents/incidents.py

@@ -62,6 +62,7 @@ def get_resolved_team_incidents(
         resolved_pr_incidents = self.get_team_pr_incidents(team_id, interval, pr_filter)
 
         total_incidents = resolved_incidents + resolved_pr_incidents
+        total_incidents = sorted(total_incidents, key=lambda x: x.creation_date)
 
         return {incident.key: incident for incident in total_incidents}.values()
 
@@ -85,6 +86,7 @@ def get_team_incidents(
         )
 
         total_incidents = incidents + pr_incidents
+        total_incidents = sorted(total_incidents, key=lambda x: x.creation_date)
 
         return {incident.key: incident for incident in total_incidents}.values()
 
@@ -105,58 +107,62 @@ def get_team_pr_incidents(
             tr.org_repo_id
             for tr in self._code_repo_service.get_active_team_repos_by_team_id(team_id)
         )
-
-        prs = self._code_repo_service.get_prs_merged_in_interval(
-            team_repo_ids, interval, pr_filter
-        )
-
         resolution_prs_interval = Interval(
             from_time=interval.from_time, to_time=time_now()
         )
-        pr_filter: PRFilter = apply_pr_filter(
+        resolution_prs_filter: PRFilter = apply_pr_filter(
             asdict(pr_filter),
             EntityType.TEAM,
             team_id,
             [SettingType.EXCLUDED_PRS_SETTING, SettingType.INCIDENT_PRS_SETTING],
         )
+
         resolution_prs = self._code_repo_service.get_prs_merged_in_interval(
-            team_repo_ids, resolution_prs_interval, pr_filter
+            team_repo_ids, resolution_prs_interval, resolution_prs_filter
         )
 
+        pr_numbers: List[str] = []
         pr_incidents: List[Incident] = []
         repo_id_to_pr_number_to_pr_map: Dict[str, Dict[str, PullRequest]] = {}
 
-        for pr in prs:
-            if str(pr.repo_id) not in repo_id_to_pr_number_to_pr_map:
-                repo_id_to_pr_number_to_pr_map[str(pr.repo_id)] = {}
-            repo_id_to_pr_number_to_pr_map[str(pr.repo_id)][pr.number] = pr
-
         for pr in resolution_prs:
             for filter in incident_prs_setting.filters:
-                pr_number = self._extract_pr_number_from_regex(
+                incident_pr_number = self._extract_pr_number_from_regex(
                     getattr(pr, filter["field"]), filter["value"]
                 )
-                if (
-                    pr_number
-                    and str(pr.repo_id) in repo_id_to_pr_number_to_pr_map
-                    and pr_number in repo_id_to_pr_number_to_pr_map[str(pr.repo_id)]
-                ):
-                    original_pr = repo_id_to_pr_number_to_pr_map[str(pr.repo_id)][
-                        pr_number
-                    ]
-                    adapted_incident_pr = IncidentPRAdapter.adapt(original_pr, pr)
-                    pr_incidents.append(adapted_incident_pr)
-                    repo_id_to_pr_number_to_pr_map[str(pr.repo_id)].pop(
-                        original_pr.number
-                    )
+
+                if incident_pr_number:
+                    pr_numbers.append(incident_pr_number)
+                    if str(pr.repo_id) not in repo_id_to_pr_number_to_pr_map:
+                        repo_id_to_pr_number_to_pr_map[str(pr.repo_id)] = {}
+                    repo_id_to_pr_number_to_pr_map[str(pr.repo_id)][
+                        incident_pr_number
+                    ] = pr
                     break
 
+        prs = self._code_repo_service.get_prs_merged_in_interval_by_numbers(
+            list(repo_id_to_pr_number_to_pr_map.keys()), interval, pr_numbers, pr_filter
+        )
+
+        for pr in prs:
+            if (
+                str(pr.repo_id) not in repo_id_to_pr_number_to_pr_map
+                or pr.number not in repo_id_to_pr_number_to_pr_map[str(pr.repo_id)]
+            ):
+                continue
+
+            resolution_pr = repo_id_to_pr_number_to_pr_map[str(pr.repo_id)][pr.number]
+            adapted_pr_incident = IncidentPRAdapter.adapt(pr, resolution_pr)
+            pr_incidents.append(adapted_pr_incident)
+
         return pr_incidents
 
     def _extract_pr_number_from_regex(
         self, text: str, regex_pattern: str
     ) -> Optional[str]:
-        if regex_pattern and check_regex(regex_pattern):
+        if not text or not regex_pattern:
+            return None
+        if check_regex(regex_pattern):
             match = re.search(regex_pattern, text)
             if match and len(match.groups()) >= 1:
                 return match.group(1)

backend/analytics_server/mhq/store/repos/code.py

@@ -307,6 +307,25 @@ def get_prs_merged_in_interval(
 
         return query.all()
 
+    @rollback_on_exc
+    def get_prs_merged_in_interval_by_numbers(
+        self,
+        repo_ids: List[str],
+        interval: Interval,
+        numbers: List[str],
+        pr_filter: PRFilter = None,
+    ) -> List[PullRequest]:
+        query = self._db.session.query(PullRequest).options(defer(PullRequest.data))
+
+        query = self._filter_prs_by_repo_ids(query, repo_ids)
+        query = self._filter_prs_merged_in_interval(query, interval)
+        query = self._filter_prs(query, pr_filter)
+        query = query.filter(PullRequest.number.in_(numbers))
+
+        query = query.order_by(PullRequest.state_changed_at.asc())
+
+        return query.all()
+
     @rollback_on_exc
     def get_pull_request_by_id(self, pr_id: str) -> PullRequest:
         return (

backend/analytics_server/tests/service/Incidents/test_team_pr_incidents.py

@@ -0,0 +1,236 @@
+from datetime import datetime
+from unittest.mock import patch, Mock
+
+import pytz
+from mhq.service.incidents.incidents import IncidentService, get_incident_service
+from mhq.utils.time import Interval
+from mhq.service.settings.models import IncidentPRsSetting
+from mhq.store.models.settings.configuration_settings import SettingType
+from mhq.store.models.code.filter import PRFilter
+from mhq.store.models.code import TeamRepos, PullRequest
+from mhq.service.settings.models import ConfigurationSettings
+from mhq.store.models import EntityType
+import pytest
+
+
+def mock_interval():
+    start_time = datetime(2025, 1, 1, 0, 0, 0, tzinfo=pytz.UTC)
+    end_time = datetime(2025, 3, 31, 0, 0, 0, tzinfo=pytz.UTC)
+    return Interval(start_time, end_time)
+
+
+@pytest.fixture(autouse=True)
+def mock_apply_pr_filter():
+    with patch("mhq.service.incidents.incidents.apply_pr_filter") as mock:
+        mock.return_value = PRFilter()
+        yield mock
+
+
+class FakeSettingsService:
+    def get_settings(self, *args, **kwargs):
+        filters = [
+            {
+                "field": "head_branch",
+                "value": "^revert-(\\d+)$",
+            },
+            {
+                "field": "title",
+                "value": "^Revert PR #(\\d+).*",
+            },
+        ]
+        return ConfigurationSettings(
+            entity_id="team_1",
+            entity_type=EntityType.TEAM,
+            specific_settings=IncidentPRsSetting(
+                include_revert_prs=True, filters=filters
+            ),
+            updated_by="user_1",
+            created_at=datetime(2025, 1, 1, 0, 0, 0, tzinfo=pytz.UTC),
+            updated_at=datetime(2025, 1, 1, 0, 0, 0, tzinfo=pytz.UTC),
+        )
+
+    def get_settings_map(self, *args, **kwargs):
+        return {
+            SettingType.INCIDENT_PRS_SETTING: self.get_settings(*args, **kwargs),
+        }
+
+
+class FakeCodeRepoService:
+    def __init__(self, prs_using_filters, prs_using_numbers):
+        self._prs_using_filters = prs_using_filters
+        self._prs_using_numbers = prs_using_numbers
+
+    def get_active_team_repos_by_team_id(self, *args, **kwargs):
+        return [
+            TeamRepos(
+                team_id="team_1",
+                org_repo_id="repo_1",
+            ),
+            TeamRepos(
+                team_id="team_1",
+                org_repo_id="repo_2",
+            ),
+        ]
+
+    def get_prs_merged_in_interval(self, *args, **kwargs):
+        return self._prs_using_filters
+
+    def get_prs_merged_in_interval_by_numbers(self, *args, **kwargs):
+        return self._prs_using_numbers
+
+
+class FakeIncidentsRepoService:
+    pass
+
+
+def test_get_team_pr_incidents_no_filters():
+    incident_service = get_incident_service()
+
+    mock_settings_service = Mock()
+    mock_settings_service.get_settings.return_value.specific_settings = (
+        IncidentPRsSetting(include_revert_prs=True, filters=[])
+    )
+    incident_service._settings_service = mock_settings_service
+
+    result = incident_service.get_team_pr_incidents(
+        "team_1",
+        mock_interval(),
+        PRFilter(),
+    )
+
+    assert result == []
+
+
+def test_get_team_pr_incidents_with_filters():
+
+    prs_using_filters = [
+        PullRequest(
+            id="pr_2_of_repo_1",
+            repo_id="repo_1",
+            number="2",
+            head_branch="revert-1",
+        ),
+        PullRequest(
+            id="pr_4_of_repo_1",
+            repo_id="repo_1",
+            number="4",
+            head_branch="branch_4",
+            title="Revert PR #3 due to some reason",
+        ),
+    ]
+
+    prs_using_numbers = [
+        PullRequest(
+            id="pr_1_of_repo_1", repo_id="repo_1", number="1", head_branch="branch_1"
+        ),
+        PullRequest(
+            id="pr_3_of_repo_1",
+            repo_id="repo_1",
+            number="3",
+            head_branch="branch_3",
+        ),
+    ]
+
+    incident_service = IncidentService(
+        FakeIncidentsRepoService(),
+        FakeSettingsService(),
+        FakeCodeRepoService(prs_using_filters, prs_using_numbers),
+    )
+
+    expected_result_keys = [
+        "pr_1_of_repo_1",
+        "pr_3_of_repo_1",
+    ]
+
+    result = incident_service.get_team_pr_incidents(
+        "team_1",
+        mock_interval(),
+        PRFilter(),
+    )
+
+    assert expected_result_keys == [incident.key for incident in result]
+
+
+def test_get_team_pr_incidents_with_multiple_repos_but_no_incidents():
+
+    prs_using_filters = [
+        PullRequest(
+            id="pr_2_of_repo_1",
+            repo_id="repo_1",
+            number="2",
+            head_branch="revert-1",
+        ),
+        PullRequest(
+            id="pr_2_of_repo_2",
+            repo_id="repo_2",
+            number="2",
+            head_branch="revert-1",
+        ),
+    ]
+
+    prs_using_numbers = []
+
+    incident_service = IncidentService(
+        FakeIncidentsRepoService(),
+        FakeSettingsService(),
+        FakeCodeRepoService(prs_using_filters, prs_using_numbers),
+    )
+
+    expected_result_keys = []
+
+    result = incident_service.get_team_pr_incidents(
+        "team_1",
+        mock_interval(),
+        PRFilter(),
+    )
+
+    assert expected_result_keys == [incident.key for incident in result]
+
+
+def test_get_team_pr_incidents_with_multiple_repos_and_incidents():
+
+    prs_using_filters = [
+        PullRequest(
+            id="pr_2_of_repo_1", repo_id="repo_1", number="2", head_branch="revert-1"
+        ),
+        PullRequest(
+            id="pr_2_of_repo_2",
+            repo_id="repo_2",
+            number="2",
+            head_branch="revert-1",
+        ),
+    ]
+
+    prs_using_numbers = [
+        PullRequest(
+            id="pr_1_of_repo_1",
+            repo_id="repo_1",
+            number="1",
+            head_branch="branch_1",
+        ),
+        PullRequest(
+            id="pr_1_of_repo_2",
+            repo_id="repo_2",
+            number="1",
+            head_branch="branch_1",
+        ),
+    ]
+
+    incident_service = IncidentService(
+        FakeIncidentsRepoService(),
+        FakeSettingsService(),
+        FakeCodeRepoService(prs_using_filters, prs_using_numbers),
+    )
+
+    expected_result_keys = [
+        "pr_1_of_repo_1",
+        "pr_1_of_repo_2",
+    ]
+
+    result = incident_service.get_team_pr_incidents(
+        "team_1",
+        mock_interval(),
+        PRFilter(),
+    )
+
+    assert expected_result_keys == [incident.key for incident in result]