Add the PR commented state (#40)

.github/workflows/run_unit_tests.yml

@@ -0,0 +1,27 @@
+---
+name: Python Tests
+
+on: [push, pull_request]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v4
+        with:
+          python-version: 3.x
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements.txt
+          pip install pytest pytest-cov
+
+      - name: Run tests with coverage (configuration in setup.cfg)
+        run: |
+          pytest

.gitignore

@@ -2,6 +2,7 @@
 __pycache__/
 .env
 .mypy_cache
+build/
 venv/
 .idea
 *.egg-info

README.md

@@ -28,10 +28,11 @@ Slack API Token with following permissions
 | emoji                        | description                                                  |
 |------------------------------|--------------------------------------------------------------|
 | `SLAPR_EMOJI_REVIEW_STARTED` | The PR has at least 1 in-progress review.                    |
-| `SLAPR_EMOJI_APPROVED`             | The PR has all required approvals and is ready to be merged. |
-| `SLAPR_EMOJI_NEEDS_CHANGES`        | Changes are requested for the PR.                            |
-| `SLAPR_EMOJI_MERGED`               | The PR is merged.                                            |
-| `SLAPR_EMOJI_CLOSED`               | The PR is closed.                                            |
+| `SLAPR_EMOJI_APPROVED`       | The PR has all required approvals and is ready to be merged. |
+| `SLAPR_EMOJI_NEEDS_CHANGES`  | Changes are requested for the PR.                            |
+| `SLAPR_EMOJI_COMMENTED`      | A review has been submitted with comment only.               |
+| `SLAPR_EMOJI_MERGED`         | The PR is merged.                                            |
+| `SLAPR_EMOJI_CLOSED`         | The PR is closed.                                            |
 
 ## Example Usage
 

slapr/__main__.py

@@ -30,6 +30,7 @@
     emoji_needs_change=os.environ.get("SLAPR_EMOJI_CHANGES_REQUESTED", "changes_requested"),
     emoji_merged=os.environ.get("SLAPR_EMOJI_MERGED", "merged"),
     emoji_closed=os.environ.get("SLAPR_EMOJI_CLOSED", "closed"),
+    emoji_commented=os.environ.get("SLAPR_EMOJI_COMMENTED", "comment"),
 )
 
 main(config)

slapr/config.py

@@ -23,6 +23,7 @@ class Config(NamedTuple):
     emoji_needs_change: str
     emoji_merged: str
     emoji_closed: str
+    emoji_commented: str
 
     @property
     def emojis_by_review_step(self) -> Callable[[str], int]:
@@ -32,6 +33,7 @@ def emojis_by_review_step(self) -> Callable[[str], int]:
         """
         review_steps_as_emojis = [
             self.emoji_review_started,
+            self.emoji_commented,
             self.emoji_needs_change,
             self.emoji_approved,
             self.emoji_closed,

slapr/emojis.py

@@ -3,34 +3,37 @@
 # This product includes software developed at Datadog (https://www.datadoghq.com/)
 # Copyright 2023-present Datadog, Inc.
 
-from typing import List, Optional, Set, Tuple
-
 import itertools
+from typing import List, Optional, Set, Tuple
 
 from .github import Review
 
 
-def get_for_reviews(reviews: List[Review], emoji_needs_change: str, emoji_approved: str, number_of_approvals_required: int) -> Optional[str]:
-    reviews_without_comments = [review for review in reviews if review.state != "commented"]
+def get_for_reviews(
+    reviews: List[Review],
+    emoji_commented: int,
+    emoji_needs_change: str,
+    emoji_approved: str,
+    number_of_approvals_required: int,
+) -> Optional[str]:
 
     reviews_by_author = {
-        username: list(reviews)
-        for username, reviews in itertools.groupby(reviews_without_comments, key=lambda review: review.username)
+        username: list(reviews) for username, reviews in itertools.groupby(reviews, key=lambda review: review.username)
     }
-
-    approvals = [review.state for review in reviews_without_comments]
-    approvalsCount = approvals.count("approved")
 
     last_reviews = [reviews[-1] for reviews in reviews_by_author.values() if reviews]
-
     unique_states = {review.state for review in last_reviews}
 
     if "changes_requested" in unique_states:
         return emoji_needs_change
 
-    if ("approved" in unique_states) and approvalsCount >= number_of_approvals_required:
+    approval_count = len([review.state for review in reviews if review.state == "approved"])
+    if ("approved" in unique_states) and approval_count >= number_of_approvals_required:
         return emoji_approved
 
+    if "commented" in unique_states:
+        return emoji_commented
+
     return None
 
 

slapr/main.py

@@ -3,8 +3,8 @@
 # This product includes software developed at Datadog (https://www.datadoghq.com/)
 # Copyright 2023-present Datadog, Inc.
 
-from .config import Config
 from . import emojis
+from .config import Config
 
 
 def main(config: Config) -> None:
@@ -23,7 +23,11 @@ def main(config: Config) -> None:
     pr = github.get_pr(pr_number=pr_number)
     reviews = github.get_pr_reviews(pr_number=pr_number)
     review_emoji = emojis.get_for_reviews(
-        reviews, emoji_needs_change=config.emoji_needs_change, emoji_approved=config.emoji_approved, number_of_approvals_required=config.number_of_approvals_required
+        reviews,
+        emoji_commented=config.emoji_commented,
+        emoji_needs_change=config.emoji_needs_change,
+        emoji_approved=config.emoji_approved,
+        number_of_approvals_required=config.number_of_approvals_required,
     )
 
     pr_url: str = event["pull_request"]["html_url"]
@@ -52,7 +56,7 @@ def main(config: Config) -> None:
 
     if pr.merged:
         new_emojis.add(config.emoji_merged)
-    elif pr.state == 'closed':
+    elif pr.state == "closed":
         new_emojis.add(config.emoji_closed)
 
     # Add emojis
@@ -65,10 +69,14 @@ def main(config: Config) -> None:
 
     for review_emoji in sorted_emojis_to_add:
         slack.add_reaction(
-            timestamp=timestamp, emoji=review_emoji, channel_id=config.slack_channel_id,
+            timestamp=timestamp,
+            emoji=review_emoji,
+            channel_id=config.slack_channel_id,
         )
 
     for review_emoji in emojis_to_remove:
         slack.remove_reaction(
-            timestamp=timestamp, emoji=review_emoji, channel_id=config.slack_channel_id,
+            timestamp=timestamp,
+            emoji=review_emoji,
+            channel_id=config.slack_channel_id,
         )

tests/test_slapr.py

@@ -61,11 +61,7 @@ def get_pr_reviews(self, pr_number: int) -> List[Review]:
     "pull_request": {
         "number": 42,
         "html_url": "https://github.com/example/repo/pull/42",
-        "head": {
-            "repo": {
-                "fork": False
-            }
-        }
+        "head": {"repo": {"fork": False}},
     }
 }
 
@@ -89,9 +85,9 @@ def get_pr_reviews(self, pr_number: int) -> List[Review]:
         ),
         pytest.param(
             [Message(text="Need :eyes: <https://github.com/example/repo/pull/42>", timestamp="yyyy-mm-dd")],
-            [Review(state="comment", username="alice")],
+            [Review(state="commented", username="alice")],
             [],
-            ["test_review_started"],
+            ["test_review_started", "test_commented"],
             id="comment",
         ),
         pytest.param(
@@ -101,6 +97,27 @@ def get_pr_reviews(self, pr_number: int) -> List[Review]:
             ["test_review_started", "test_approved"],
             id="approved-from-changes-requested",
         ),
+        pytest.param(
+            [Message(text="Need :eyes: <https://github.com/example/repo/pull/42>", timestamp="yyyy-mm-dd")],
+            [Review(state="commented", username="alice"), Review(state="approved", username="alice")],
+            [Reaction(emoji="test_commented", user_ids=["U1234"])],
+            ["test_review_started", "test_approved"],
+            id="approved-from-commented",
+        ),
+        pytest.param(
+            [Message(text="Need :eyes: <https://github.com/example/repo/pull/42>", timestamp="yyyy-mm-dd")],
+            [Review(state="changes_requested", username="alice"), Review(state="comment", username="bob")],
+            [],
+            ["test_review_started", "test_needs_change"],
+            id="commented-ignored-when-changes-requested",
+        ),
+        pytest.param(
+            [Message(text="Need :eyes: <https://github.com/example/repo/pull/42>", timestamp="yyyy-mm-dd")],
+            [Review(state="changes_requested", username="alice"), Review(state="approved", username="bob")],
+            [],
+            ["test_review_started", "test_needs_change"],
+            id="approved-ignored-when-changes-requested",
+        ),
         pytest.param(
             [Message(text="Need :eyes: but I've got no PR URL", timestamp="yyyy-mm-dd")],
             [Review(state="approved", username="alice")],
@@ -131,6 +148,7 @@ def test_on_pull_request_review(
         emoji_needs_change="test_needs_change",
         emoji_merged="test_merged",
         emoji_closed="test_closed",
+        emoji_commented="test_commented",
     )
     slapr.main(config)
 
@@ -167,7 +185,11 @@ def test_on_pull_request(event: dict, pr: PullRequest, reactions: List[Reaction]
     reviews = [Review(state="approved", username="alice")]
 
     slack_backend = MockSlackBackend(messages=messages, target_message=messages[0], reactions=reactions)
-    github_backend = MockGithubBackend(reviews=reviews, event=event, pr=pr,)
+    github_backend = MockGithubBackend(
+        reviews=reviews,
+        event=event,
+        pr=pr,
+    )
 
     config = Config(
         slack_client=SlackClient(backend=slack_backend),
@@ -180,6 +202,7 @@ def test_on_pull_request(event: dict, pr: PullRequest, reactions: List[Reaction]
         emoji_needs_change="test_needs_change",
         emoji_merged="test_merged",
         emoji_closed="test_closed",
+        emoji_commented="test_commented",
     )
     slapr.main(config)