fix: Create GitHub comment as table (#3948)

Author: novakzaballa

api/conftest.py

@@ -553,6 +553,29 @@ def segment_featurestate(feature_segment, feature, environment):
     )
 
 
+@pytest.fixture()
+def feature_with_value_segment(
+    feature_with_value: Feature, segment: Segment, environment: Environment
+) -> FeatureSegment:
+    return FeatureSegment.objects.create(
+        feature=feature_with_value, segment=segment, environment=environment
+    )
+
+
+@pytest.fixture()
+def segment_featurestate_and_feature_with_value(
+    feature_with_value_segment: FeatureSegment,
+    feature_with_value: Feature,
+    environment: Environment,
+) -> FeatureState:
+    return FeatureState.objects.create(
+        feature_segment=feature_with_value_segment,
+        feature=feature_with_value,
+        environment=environment,
+        updated_at="2024-01-01 00:00:00",
+    )
+
+
 @pytest.fixture()
 def environment_api_key(environment):
     return EnvironmentAPIKey.objects.create(

api/features/feature_external_resources/models.py

@@ -61,11 +61,12 @@ def execute_after_save_actions(self):
                 feature_id=self.feature_id, identity_id__isnull=True
             )
             feature_data: GithubData = generate_data(
-                github_configuration,
-                self.feature_id,
-                self.feature.name,
-                WebhookEventType.FEATURE_EXTERNAL_RESOURCE_ADDED.value,
-                feature_states,
+                github_configuration=github_configuration,
+                feature_id=self.feature_id,
+                feature_name=self.feature.name,
+                type=WebhookEventType.FEATURE_EXTERNAL_RESOURCE_ADDED.value,
+                feature_states=feature_states,
+                project_id=self.feature.project_id,
             )
 
             call_github_app_webhook_for_feature_state.delay(

api/features/models.py

@@ -1014,7 +1014,6 @@ def create_github_comment(self) -> None:
 
         if (
             not self.identity_id
-            and not self.feature_segment
             and self.feature.external_resources.exists()
             and self.environment.project.github_project.exists()
             and self.environment.project.organisation.github_config.exists()
@@ -1032,6 +1031,7 @@ def create_github_comment(self) -> None:
                     feature_name=self.feature.name,
                     type=WebhookEventType.FLAG_UPDATED.value,
                     feature_states=feature_states,
+                    project_id=self.environment.project_id,
                 )
 
             if self.deleted_at is not None:

api/integrations/github/constants.py

@@ -1,8 +1,13 @@
 GITHUB_API_URL = "https://api.github.com/"
 GITHUB_API_VERSION = "2022-11-28"
 
-LINK_FEATURE_TEXT = "### This pull request is linked to a Flagsmith Feature (`%s`):\n"
+LINK_FEATURE_TITLE = """**Flagsmith feature linked:** `%s`
+Default Values:\n"""
+FEATURE_TABLE_HEADER = """| Environment | Enabled | Value | Last Updated (UTC) |
+| :--- | :----- | :------ | :------ |\n"""
+FEATURE_TABLE_ROW = "| [%s](%s) | %s | %s | %s |\n"
+LINK_SEGMENT_TITLE = "Segment `%s` values:\n"
 UNLINKED_FEATURE_TEXT = "### The feature flag `%s` was unlinked from the issue/PR"
-UPDATED_FEATURE_TEXT = "### The Flagsmith Feature `%s` was updated in the environment "
-LAST_UPDATED_FEATURE_TEXT = "Last Updated %s"
+UPDATED_FEATURE_TEXT = "Flagsmith Feature `%s` has been updated:\n"
 DELETED_FEATURE_TEXT = "### The Feature Flag `%s` was deleted"
+FEATURE_ENVIRONMENT_URL = "%s/project/%s/environment/%s/features?feature=%s&tab=%s"

api/integrations/github/github.py

@@ -1,18 +1,21 @@
-import datetime
 import logging
 import typing
 from dataclasses import dataclass
 
 import requests
+from core.helpers import get_current_site_url
 from django.conf import settings
 
 from features.models import FeatureState, FeatureStateValue
 from integrations.github.client import generate_token
 from integrations.github.constants import (
     DELETED_FEATURE_TEXT,
+    FEATURE_ENVIRONMENT_URL,
+    FEATURE_TABLE_HEADER,
+    FEATURE_TABLE_ROW,
     GITHUB_API_URL,
-    LAST_UPDATED_FEATURE_TEXT,
-    LINK_FEATURE_TEXT,
+    LINK_FEATURE_TITLE,
+    LINK_SEGMENT_TITLE,
     UNLINKED_FEATURE_TEXT,
     UPDATED_FEATURE_TEXT,
 )
@@ -28,8 +31,9 @@ class GithubData:
     feature_id: int
     feature_name: str
     type: str
-    feature_states: typing.List[dict[str, typing.Any]] = None
-    url: str = None
+    feature_states: typing.List[dict[str, typing.Any]] | None = None
+    url: str | None = None
+    project_id: int | None = None
 
     @classmethod
     def from_dict(cls, data_dict: dict) -> "GithubData":
@@ -65,6 +69,8 @@ def post_comment_to_github(
 def generate_body_comment(
     name: str,
     event_type: str,
+    project_id: int,
+    feature_id: int,
     feature_states: typing.List[typing.Dict[str, typing.Any]],
 ) -> str:
 
@@ -78,25 +84,34 @@ def generate_body_comment(
     if is_removed:
         return delete_text
 
-    last_updated_string = LAST_UPDATED_FEATURE_TEXT % (
-        datetime.datetime.now().strftime("%dth %b %Y %I:%M%p")
-    )
-    updated_text = UPDATED_FEATURE_TEXT % (name)
-
-    result = updated_text if is_update else LINK_FEATURE_TEXT % (name)
+    result = UPDATED_FEATURE_TEXT % (name) if is_update else LINK_FEATURE_TITLE % (name)
+    last_segment_name = ""
+    if len(feature_states) > 0 and not feature_states[0].get("segment_name"):
+        result += FEATURE_TABLE_HEADER
 
-    # if feature_states is None:
     for v in feature_states:
         feature_value = v.get("feature_state_value")
-        feature_value_type = v.get("feature_state_value_type")
-
-        feature_value_string = f"\n{feature_value_type}\n```{feature_value if feature_value else 'No value.'}```\n\n"
-
-        result += f"**{v['environment_name']}{' - ' + v['segment_name'] if v.get('segment_name') else ''}**\n"
-        result += f"- [{'x' if v['feature_value'] else ' '}] {'Enabled' if v['feature_value'] else 'Disabled'}"
-        result += feature_value_string
+        tab = "segment-overrides" if v.get("segment_name") is not None else "value"
+        environment_link_url = FEATURE_ENVIRONMENT_URL % (
+            get_current_site_url(),
+            project_id,
+            v.get("environment_api_key"),
+            feature_id,
+            tab,
+        )
+        if v.get("segment_name") is not None and v["segment_name"] != last_segment_name:
+            result += "\n" + LINK_SEGMENT_TITLE % (v["segment_name"])
+            last_segment_name = v["segment_name"]
+            result += FEATURE_TABLE_HEADER
+        table_row = FEATURE_TABLE_ROW % (
+            v["environment_name"],
+            environment_link_url,
+            "✅ Enabled" if v["enabled"] else "❌ Disabled",
+            f"`{feature_value}`" if feature_value else "",
+            v["last_updated"],
+        )
+        result += table_row
 
-    result += last_updated_string
     return result
 
 
@@ -109,8 +124,11 @@ def generate_data(
     feature_id: int,
     feature_name: str,
     type: str,
-    feature_states: typing.Union[list[FeatureState], list[FeatureStateValue]] = None,
-    url: str = None,
+    feature_states: (
+        typing.Union[list[FeatureState], list[FeatureStateValue]] | None
+    ) = None,
+    url: str | None = None,
+    project_id: int | None = None,
 ) -> GithubData:
 
     if feature_states:
@@ -126,7 +144,11 @@ def generate_data(
                 feature_env_data["feature_state_value_type"] = feature_state_value_type
             if type is not WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value:
                 feature_env_data["environment_name"] = feature_state.environment.name
-                feature_env_data["feature_value"] = feature_state.enabled
+                feature_env_data["enabled"] = feature_state.enabled
+                feature_env_data["last_updated"] = feature_state.updated_at
+                feature_env_data["environment_api_key"] = (
+                    feature_state.environment.api_key
+                )
             if (
                 hasattr(feature_state, "feature_segment")
                 and feature_state.feature_segment is not None
@@ -147,4 +169,5 @@ def generate_data(
             else None
         ),
         feature_states=feature_states_list if feature_states else None,
+        project_id=project_id,
     )

api/integrations/github/tasks.py

@@ -24,12 +24,16 @@ class CallGithubData:
 
 def send_post_request(data: CallGithubData) -> None:
     feature_name = data.github_data.feature_name
+    feature_id = data.github_data.feature_id
+    project_id = data.github_data.project_id
     event_type = data.event_type
     feature_states = feature_states = (
         data.github_data.feature_states if data.github_data.feature_states else None
     )
     installation_id = data.github_data.installation_id
-    body = generate_body_comment(feature_name, event_type, feature_states)
+    body = generate_body_comment(
+        feature_name, event_type, project_id, feature_id, feature_states
+    )
 
     if (
         event_type == WebhookEventType.FLAG_UPDATED.value

api/tests/unit/features/test_unit_feature_external_resources_views.py

@@ -1,10 +1,8 @@
-import datetime
-
 import pytest
 import simplejson as json
 from django.core.serializers.json import DjangoJSONEncoder
 from django.urls import reverse
-from freezegun import freeze_time
+from pytest_mock import MockerFixture
 from rest_framework import status
 from rest_framework.test import APIClient
 
@@ -37,24 +35,44 @@ def json(self):
     return MockResponse(json_data={"data": "data"}, status_code=200)
 
 
-@freeze_time("2024-01-01")
 def test_create_feature_external_resource(
     admin_client_new: APIClient,
     feature_with_value: Feature,
+    segment_featurestate_and_feature_with_value: FeatureState,
+    environment: Environment,
     project: Project,
     github_configuration: GithubConfiguration,
     github_repository: GithubRepository,
-    mocker,
+    mocker: MockerFixture,
 ) -> None:
     # Given
     mock_generate_token = mocker.patch(
         "integrations.github.github.generate_token",
     )
+
     mock_generate_token.return_value = "mocked_token"
     github_request_mock = mocker.patch(
         "requests.post", side_effect=mocked_requests_post
     )
-    datetime_now = datetime.datetime.now()
+
+    feature_state = FeatureState.objects.filter(feature=feature_with_value).first()
+
+    expected_comment_body = (
+        "**Flagsmith feature linked:** `feature_with_value`\n"
+        "Default Values:\n"
+        "| Environment | Enabled | Value | Last Updated (UTC) |\n"
+        "| :--- | :----- | :------ | :------ |\n"
+        f"| [Test Environment](https://example.com/project/{project.id}/"
+        f"environment/{environment.api_key}/features?feature={feature_with_value.id}&tab=value) "
+        f"| ❌ Disabled | `value` | {feature_state.updated_at} |\n"
+        "\n"
+        "Segment `segment` values:\n"
+        "| Environment | Enabled | Value | Last Updated (UTC) |\n"
+        "| :--- | :----- | :------ | :------ |\n"
+        f"| [Test Environment](https://example.com/project/{project.id}/"
+        f"environment/{environment.api_key}/features?feature={feature_with_value.id}&tab=segment-overrides) "
+        f"| ❌ Disabled | `value` | {segment_featurestate_and_feature_with_value.updated_at} |\n"
+    )
 
     feature_external_resource_data = {
         "type": "GITHUB_ISSUE",
@@ -76,9 +94,7 @@ def test_create_feature_external_resource(
     # Then
     github_request_mock.assert_called_with(
         "https://api.github.com/repos/repoowner/repo-name/issues/35/comments",
-        json={
-            "body": f"### This pull request is linked to a Flagsmith Feature (`feature_with_value`):\n**Test Environment**\n- [ ] Disabled\nunicode\n```value```\n\nLast Updated {datetime_now.strftime('%dth %b %Y %I:%M%p')}"  # noqa E501
-        },
+        json={"body": f"{expected_comment_body}"},
         headers={
             "Accept": "application/vnd.github.v3+json",
             "Authorization": "Bearer mocked_token",
@@ -320,9 +336,10 @@ def test_create_github_comment_on_feature_state_updated(  # noqa: C901
     with_environment_permissions: WithEnvironmentPermissionsCallable,
     feature_external_resource: FeatureExternalResource,
     feature: Feature,
+    project: Project,
     github_configuration: GithubConfiguration,
     github_repository: GithubRepository,
-    mocker,
+    mocker: MockerFixture,
     environment: Environment,
     event_type: str,
 ) -> None:
@@ -412,4 +429,5 @@ def test_create_github_comment_on_feature_state_updated(  # noqa: C901
             feature_name=feature.name,
             type=WebhookEventType.FLAG_UPDATED.value,
             feature_states=[feature_state],
+            project_id=project.id,
         )