diff --git a/backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py b/backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py index 88290691..18f38922 100644 --- a/backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py +++ b/backend/analytics_server/mhq/service/code/sync/etl_code_analytics.py @@ -23,10 +23,8 @@ def create_pr_metrics( ) -> PullRequest: if pr.state == PullRequestState.OPEN: return pr - non_bot_pr_events = self.filter_non_bot_events(pr_events) pr_performance = self.get_pr_performance(pr, non_bot_pr_events) - pr.first_response_time = ( pr_performance.first_review_time if pr_performance.first_review_time != -1 @@ -60,7 +58,6 @@ def create_pr_metrics( @staticmethod def get_pr_performance(pr: PullRequest, pr_events: [PullRequestEvent]): - review_events = [ event for event in pr_events @@ -92,8 +89,14 @@ def get_pr_performance(pr: PullRequest, pr_events: [PullRequestEvent]): ) ) + first_response_end_time = ( + first_review.created_at if first_review else pr.state_changed_at + ) + if not approved_reviews: - rework_time = -1 + rework_time = ( + pr.state_changed_at - first_response_end_time + ).total_seconds() else: if first_review.data.get("state") == PullRequestEventState.APPROVED.value: rework_time = 0 @@ -102,8 +105,10 @@ def get_pr_performance(pr: PullRequest, pr_events: [PullRequestEvent]): approved_reviews[0].created_at - first_review.created_at ).total_seconds() - if pr.state != PullRequestState.MERGED or not approved_reviews: + if pr.state != PullRequestState.MERGED: merge_time = -1 + elif not approved_reviews: + merge_time = 0 else: merge_time = ( pr.state_changed_at - approved_reviews[0].created_at @@ -124,9 +129,9 @@ def get_pr_performance(pr: PullRequest, pr_events: [PullRequestEvent]): return PRPerformance( first_review_time=( ( - first_review.created_at - pull_request_ready_for_review_time + first_response_end_time - pull_request_ready_for_review_time ).total_seconds() - if first_review + if first_response_end_time else -1 ), rework_time=rework_time, diff --git a/backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py b/backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py index 3ae87bd8..d4754c6e 100644 --- a/backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py +++ b/backend/analytics_server/tests/service/code/sync/test_etl_code_analytics.py @@ -21,11 +21,15 @@ def test_pr_performance_returns_first_review_tat_for_first_review(): assert performance.first_review_time == 3600 -def test_pr_performance_returns_minus1_first_review_tat_for_no_reviews(): +def test_pr_performance_returns_first_review_time_as_fallback_for_no_reviews(): pr_service = CodeETLAnalyticsService() - pr = get_pull_request() + t1 = time_now() + t2 = t1 + timedelta(minutes=30) + pr = get_pull_request( + state=PullRequestState.MERGED, state_changed_at=t2, created_at=t1, updated_at=t2 + ) performance = pr_service.get_pr_performance(pr, []) - assert performance.first_review_time == -1 + assert performance.first_review_time == 1800.0 def test_pr_performance_returns_minus1_first_approved_review_tat_for_no_approved_review(): @@ -40,7 +44,7 @@ def test_pr_performance_returns_minus1_first_approved_review_tat_for_no_approved assert performance.merge_time == -1 -def test_pr_performance_returns_merge_time_minus1_for_merged_pr_without_review(): +def test_pr_performance_returns_merge_time_zero_for_merged_pr_without_review(): pr_service = CodeETLAnalyticsService() t1 = time_now() t2 = time_now() + timedelta(minutes=30) @@ -48,7 +52,7 @@ def test_pr_performance_returns_merge_time_minus1_for_merged_pr_without_review() state=PullRequestState.MERGED, state_changed_at=t2, created_at=t1, updated_at=t2 ) performance = pr_service.get_pr_performance(pr, []) - assert performance.merge_time == -1 + assert performance.merge_time == 0 def test_pr_performance_returns_blocking_reviews(): @@ -156,11 +160,14 @@ def test_pr_performance_returns_rework_time_for_open_prs(): assert performance.rework_time == (t3 - t2).total_seconds() -def test_pr_performance_returns_rework_time_minus1_for_non_approved_prs(): +def test_pr_performance_returns_rework_time_for_non_approved_prs(): pr_service = CodeETLAnalyticsService() t1 = time_now() t2 = t1 + timedelta(hours=1) - pr = get_pull_request(state=PullRequestState.OPEN, created_at=t1, updated_at=t1) + t3 = t2 + timedelta(hours=2) + pr = get_pull_request( + state=PullRequestState.OPEN, created_at=t1, updated_at=t1, state_changed_at=t3 + ) changes_requested_1 = get_pull_request_event( pull_request_id=pr.id, state=PullRequestEventState.CHANGES_REQUESTED.value, @@ -168,10 +175,10 @@ def test_pr_performance_returns_rework_time_minus1_for_non_approved_prs(): ) performance = pr_service.get_pr_performance(pr, [changes_requested_1]) - assert performance.rework_time == -1 + assert performance.rework_time == (t3 - t2).total_seconds() -def test_pr_performance_returns_rework_time_minus1_for_merged_prs_without_reviews(): +def test_pr_performance_returns_rework_time_zero_for_merged_prs_without_reviews(): pr_service = CodeETLAnalyticsService() t1 = time_now() pr = get_pull_request( @@ -179,7 +186,7 @@ def test_pr_performance_returns_rework_time_minus1_for_merged_prs_without_review ) performance = pr_service.get_pr_performance(pr, []) - assert performance.rework_time == -1 + assert performance.rework_time == 0.0 def test_pr_performance_returns_cycle_time_for_merged_pr():