Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Create GitHub comment as table #3948

Merged
merged 5 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,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(
Expand Down
11 changes: 6 additions & 5 deletions api/features/feature_external_resources/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion api/features/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,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()
Expand All @@ -1023,6 +1022,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:
Expand Down
11 changes: 8 additions & 3 deletions api/integrations/github/constants.py
Original file line number Diff line number Diff line change
@@ -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"
69 changes: 46 additions & 23 deletions api/integrations/github/github.py
Original file line number Diff line number Diff line change
@@ -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,
)
Expand All @@ -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":
Expand Down Expand Up @@ -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:

Expand All @@ -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 % (
Comment on lines +102 to +105
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a segment to the test or slap a # pragma: no cover on L108.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

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


Expand All @@ -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:
Expand All @@ -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
Expand All @@ -147,4 +169,5 @@ def generate_data(
else None
),
feature_states=feature_states_list if feature_states else None,
project_id=project_id,
)
6 changes: 5 additions & 1 deletion api/integrations/github/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
)
Loading