Repository: github/issue-metrics
Pull request #431 (2024-11-26 23:07:22)

fix: Update time in draft to account for edge cases and update tests
# Pull Request

<!--
PR title needs to be prefixed with a conventional commit type
(build,chore,ci,docs,feat,fix,perf,refactor,revert,style,test)

It should also be brief and descriptive for a good changelog entry

examples: "feat: add new logger" or "fix: remove unused imports"
-->

## Proposed Changes

Fixes: #417 

- Updated the time in draft to work even if a pr has been changed from `ready for review` to `draft` by using the events from the issues object. 
- Also updated the test cases to reflect and test the new changes.

![image](https://github.com/user-attachments/assets/669e66c2-424c-4675-bca0-05ce6ace6378)

<!-- Describe what the changes are and link to a GitHub Issue if one exists -->

## Readiness Checklist

### Author/Contributor

- [x] If documentation is needed for this change, has that been included in this pull request
- [x] run `make lint` and fix any issues that you have introduced
- [x] run `make test` and ensure you have test coverage for the lines you are introducing
- [ ] If publishing new data to the public (scorecards, security scan results, code quality results, live dashboards, etc.), please request review from `@jeffrey-luszcz`

![image](https://github.com/user-attachments/assets/cf392bff-77d2-40c7-b3f1-fd7fda90aeb8)

![image](https://github.com/user-attachments/assets/96be8d43-07a8-48c4-9e6a-e9de29ffda32)

### Reviewer

- [x] Label as either `fix`, `documentation`, `enhancement`, `infrastructure`, `maintenance`, or `breaking`



Patch:
diff --git a/issue_metrics.py b/issue_metrics.py
index cdbaef9..0fd3a19 100644
--- a/issue_metrics.py
+++ b/issue_metrics.py
@@ -125,8 +125,7 @@ def get_per_issue_metrics(
                 ready_for_review_at = get_time_to_ready_for_review(issue, pull_request)
                 if env_vars.draft_pr_tracking:
                     issue_with_metrics.time_in_draft = measure_time_in_draft(
-                        issue=issue,
-                        ready_for_review_at=ready_for_review_at,
+                        issue=issue
                     )
 
             if env_vars.hide_time_to_first_response is False:
diff --git a/time_in_draft.py b/time_in_draft.py
index 59fe24a..9244145 100644
--- a/time_in_draft.py
+++ b/time_in_draft.py
@@ -13,23 +13,32 @@ from classes import IssueWithMetrics
 
 def measure_time_in_draft(
     issue: github3.issues.Issue,
-    ready_for_review_at: Union[datetime, None],
-) -> Union[datetime, None]:
-    """If a pull request has had time in the draft state, return the amount of time it was in draft.
+) -> Union[timedelta, None]:
+    """If a pull request has had time in the draft state, return the cumulative amount of time it was in draft.
 
     args:
         issue (github3.issues.Issue): A GitHub issue which has been pre-qualified as a pull request.
-        ready_for_review_at (datetime | None): The time the pull request was marked as
-            ready for review.
 
     returns:
-        Union[datetime, None]: The time the pull request was in draft state.
+        Union[timedelta, None]: Total time the pull request has spent in draft state.
     """
-    if ready_for_review_at:
-        return ready_for_review_at - issue.issue.created_at
-    if issue.issue.state == "open":
-        return datetime.now(pytz.utc) - issue.issue.created_at
-    return None
+    events = issue.events()
+    draft_start = None
+    total_draft_time = timedelta(0)
+
+    for event in events:
+        if event.event == "converted_to_draft":
+            draft_start = event.created_at
+        elif event.event == "ready_for_review" and draft_start:
+            # Calculate draft time for this interval
+            total_draft_time += event.created_at - draft_start
+            draft_start = None
+
+    # If the PR is currently in draft state, calculate the time in draft up to now
+    if draft_start and issue.issue.state == "open":
+        total_draft_time += datetime.now(pytz.utc) - draft_start
+
+    return total_draft_time if total_draft_time > timedelta(0) else None
 
 
 def get_stats_time_in_draft(


Test patch:
diff --git a/test_time_in_draft.py b/test_time_in_draft.py
index 486cb9b..2475147 100644
--- a/test_time_in_draft.py
+++ b/test_time_in_draft.py
@@ -18,15 +18,23 @@ class TestMeasureTimeInDraft(unittest.TestCase):
         Setup common test data and mocks.
         """
         self.issue = MagicMock()
-        self.issue.issue.created_at = datetime(2021, 1, 1, tzinfo=pytz.utc)
         self.issue.issue.state = "open"
 
     def test_time_in_draft_with_ready_for_review(self):
         """
-        Test measure_time_in_draft when ready_for_review_at is provided.
+        Test measure_time_in_draft with one draft and review interval.
         """
-        ready_for_review_at = datetime(2021, 1, 3, tzinfo=pytz.utc)
-        result = measure_time_in_draft(self.issue, ready_for_review_at)
+        self.issue.events.return_value = [
+            MagicMock(
+                event="converted_to_draft",
+                created_at=datetime(2021, 1, 1, tzinfo=pytz.utc),
+            ),
+            MagicMock(
+                event="ready_for_review",
+                created_at=datetime(2021, 1, 3, tzinfo=pytz.utc),
+            ),
+        ]
+        result = measure_time_in_draft(self.issue)
         expected = timedelta(days=2)
         self.assertEqual(result, expected, "The time in draft should be 2 days.")
 
@@ -34,21 +42,88 @@ class TestMeasureTimeInDraft(unittest.TestCase):
         """
         Test measure_time_in_draft when ready_for_review_at is not provided and issue is still open.
         """
+        self.issue.events.return_value = [
+            MagicMock(
+                event="converted_to_draft",
+                created_at=datetime(2021, 1, 1, tzinfo=pytz.utc),
+            ),
+        ]
         now = datetime(2021, 1, 4, tzinfo=pytz.utc)
         with unittest.mock.patch("time_in_draft.datetime") as mock_datetime:
             mock_datetime.now.return_value = now
-            result = measure_time_in_draft(self.issue, None)
+            result = measure_time_in_draft(self.issue)
             expected = timedelta(days=3)
             self.assertEqual(result, expected, "The time in draft should be 3 days.")
 
+    def test_time_in_draft_multiple_intervals(self):
+        """
+        Test measure_time_in_draft with multiple draft intervals.
+        """
+        self.issue.events.return_value = [
+            MagicMock(
+                event="converted_to_draft",
+                created_at=datetime(2021, 1, 1, tzinfo=pytz.utc),
+            ),
+            MagicMock(
+                event="ready_for_review",
+                created_at=datetime(2021, 1, 3, tzinfo=pytz.utc),
+            ),
+            MagicMock(
+                event="converted_to_draft",
+                created_at=datetime(2021, 1, 5, tzinfo=pytz.utc),
+            ),
+            MagicMock(
+                event="ready_for_review",
+                created_at=datetime(2021, 1, 7, tzinfo=pytz.utc),
+            ),
+        ]
+        result = measure_time_in_draft(self.issue)
+        expected = timedelta(days=4)
+        self.assertEqual(result, expected, "The total time in draft should be 4 days.")
+
+    def test_time_in_draft_ongoing_draft(self):
+        """
+        Test measure_time_in_draft with an ongoing draft interval.
+        """
+        self.issue.events.return_value = [
+            MagicMock(
+                event="converted_to_draft",
+                created_at=datetime(2021, 1, 1, tzinfo=pytz.utc),
+            ),
+        ]
+        with unittest.mock.patch("time_in_draft.datetime") as mock_datetime:
+            mock_datetime.now.return_value = datetime(2021, 1, 4, tzinfo=pytz.utc)
+            result = measure_time_in_draft(self.issue)
+            expected = timedelta(days=3)
+            self.assertEqual(
+                result, expected, "The ongoing draft time should be 3 days."
+            )
+
+    def test_time_in_draft_no_draft_events(self):
+        """
+        Test measure_time_in_draft with no draft-related events.
+        """
+        self.issue.events.return_value = []
+        result = measure_time_in_draft(self.issue)
+        self.assertIsNone(
+            result, "The result should be None when there are no draft events."
+        )
+
     def test_time_in_draft_without_ready_for_review_and_closed(self):
         """
-        Test measure_time_in_draft when ready_for_review_at is not provided and issue is closed.
+        Test measure_time_in_draft for a closed issue with an ongoing draft and ready_for_review_at is not provided.
         """
+        self.issue.events.return_value = [
+            MagicMock(
+                event="converted_to_draft",
+                created_at=datetime(2021, 1, 1, tzinfo=pytz.utc),
+            ),
+        ]
         self.issue.issue.state = "closed"
-        result = measure_time_in_draft(self.issue, None)
+        result = measure_time_in_draft(self.issue)
         self.assertIsNone(
-            result, "The result should be None when draft was never used."
+            result,
+            "The result should be None for a closed issue with an ongoing draft.",
         )