-
Notifications
You must be signed in to change notification settings - Fork 429
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
Changes from 2 commits
198d215
ed5823f
ad480be
b7659a6
0e9d821
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,14 @@ | ||
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 | Type | Updated (UTC) | | ||
| :--- | :----- | :------ | :------- | :------ |\n""" | ||
FEATURE_TABLE_ROW = "| [%s](%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 " | ||
UPDATED_FEATURE_TEXT = "Flagsmith Feature `%s` has been updated:\n" | ||
LAST_UPDATED_FEATURE_TEXT = "Last Updated %s" | ||
DELETED_FEATURE_TEXT = "### The Feature Flag `%s` was deleted" | ||
FEATURE_ENVIRONMENT_URL = "%s/project/%s/environment/%s/features?feature=%s&tab=%s" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,15 +4,20 @@ | |
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 +33,9 @@ | |
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 +71,8 @@ | |
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: | ||
|
||
|
@@ -81,22 +89,37 @@ | |
last_updated_string = LAST_UPDATED_FEATURE_TEXT % ( | ||
datetime.datetime.now().strftime("%dth %b %Y %I:%M%p") | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of the custom datetime format. Let's consider either sticking to ISO8601 or using Django locale, i.e.: from django.utils.formats import get_format
datetime_format = get_format('DATETIME_INPUT_FORMATS')[0] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
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") | ||
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 | ||
Comment on lines
+102
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add a segment to the test or slap a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
table_row = FEATURE_TABLE_ROW % ( | ||
v["environment_name"], | ||
environment_link_url, | ||
"✅ Enabled" if v["feature_value"] else "❌ Disabled", | ||
khvn26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
feature_value if feature_value else "", | ||
khvn26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
feature_value_type, | ||
khvn26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
last_updated_string, | ||
) | ||
result += table_row | ||
|
||
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 | ||
|
||
result += last_updated_string | ||
return result | ||
|
||
|
||
|
@@ -109,8 +132,11 @@ | |
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: | ||
|
@@ -127,6 +153,9 @@ | |
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["environment_api_key"] = ( | ||
feature_state.environment.api_key | ||
) | ||
if ( | ||
hasattr(feature_state, "feature_segment") | ||
and feature_state.feature_segment is not None | ||
|
@@ -147,4 +176,5 @@ | |
else None | ||
), | ||
feature_states=feature_states_list if feature_states else None, | ||
project_id=project_id, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way, we won't have to prepend "Last Updated" to every timestamp in this column.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.