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: segment overrides stale feature state value while creating GitHub comment #3961

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
11 changes: 11 additions & 0 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,17 @@ def feature_external_resource(feature: Feature) -> FeatureExternalResource:
)


@pytest.fixture()
def feature_with_value_external_resource(
feature_with_value: Feature,
) -> FeatureExternalResource:
return FeatureExternalResource.objects.create(
url="https://github.com/userexample/example-project-repo/issues/11",
type="GITHUB_ISSUE",
feature=feature_with_value,
)


@pytest.fixture()
def github_configuration(organisation: Organisation) -> GithubConfiguration:
return GithubConfiguration.objects.create(
Expand Down
7 changes: 2 additions & 5 deletions api/features/feature_external_resources/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,9 @@ def execute_after_save_actions(self):
)
feature_data: GithubData = generate_data(
github_configuration=github_configuration,
feature_id=self.feature_id,
feature_name=self.feature.name,
feature=self.feature,
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 All @@ -85,8 +83,7 @@ def execute_before_save_actions(self) -> None:
):
feature_data: GithubData = generate_data(
github_configuration=github_configuration,
feature_id=self.feature_id,
feature_name=self.feature.name,
feature=self.feature,
type=WebhookEventType.FEATURE_EXTERNAL_RESOURCE_REMOVED.value,
url=self.url,
)
Expand Down
72 changes: 29 additions & 43 deletions api/features/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,35 @@ class Meta:
# TODO: after upgrade to Django 4.0 use UniqueConstraint()
ordering = ("id",) # explicit ordering to prevent pagination warnings

@hook(AFTER_SAVE)
def create_github_comment(self) -> None:
from integrations.github.github import GithubData, generate_data
from integrations.github.tasks import (
call_github_app_webhook_for_feature_state,
)
from webhooks.webhooks import WebhookEventType

if (
self.external_resources.exists()
and self.project.github_project.exists()
and self.project.organisation.github_config.exists()
and self.deleted_at
):
github_configuration = GithubConfiguration.objects.get(
organisation_id=self.project.organisation_id
)

feature_data: GithubData = generate_data(
github_configuration=github_configuration,
feature=self,
type=WebhookEventType.FLAG_DELETED.value,
feature_states=[],
)

call_github_app_webhook_for_feature_state.delay(
args=(asdict(feature_data),),
)

@hook(AFTER_CREATE)
def create_feature_states(self):
FeatureState.create_initial_feature_states_for_feature(feature=self)
Expand Down Expand Up @@ -1004,49 +1033,6 @@ def copy_identity_feature_states(

target_feature_state.save()

@hook(AFTER_SAVE)
def create_github_comment(self) -> None:
from integrations.github.github import GithubData, generate_data
from integrations.github.tasks import (
call_github_app_webhook_for_feature_state,
)
from webhooks.webhooks import WebhookEventType

if (
not self.identity_id
and self.feature.external_resources.exists()
and self.environment.project.github_project.exists()
and self.environment.project.organisation.github_config.exists()
):
github_configuration = GithubConfiguration.objects.get(
organisation_id=self.environment.project.organisation_id
)
feature_states = []
feature_states.append(self)

if self.deleted_at is None:
feature_data: GithubData = generate_data(
github_configuration=github_configuration,
feature_id=self.feature.id,
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:
feature_data: GithubData = generate_data(
github_configuration=github_configuration,
feature_id=self.feature.id,
feature_name=self.feature.name,
type=WebhookEventType.FLAG_DELETED.value,
feature_states=feature_states,
)

call_github_app_webhook_for_feature_state.delay(
args=(asdict(feature_data),),
)


class FeatureStateValue(
AbstractBaseFeatureValueModel,
Expand Down
32 changes: 31 additions & 1 deletion api/features/serializers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import typing
from dataclasses import asdict
from datetime import datetime

import django.core.exceptions
Expand All @@ -12,6 +13,9 @@
from environments.sdk.serializers_mixins import (
HideSensitiveFieldsSerializerMixin,
)
from integrations.github.github import GithubData, generate_data
from integrations.github.models import GithubConfiguration
from integrations.github.tasks import call_github_app_webhook_for_feature_state
from metadata.serializers import MetadataSerializer, SerializerWithMetadata
from projects.models import Project
from users.serializers import (
Expand All @@ -22,6 +26,7 @@
from util.drf_writable_nested.serializers import (
DeleteBeforeUpdateWritableNestedModelSerializer,
)
from webhooks.webhooks import WebhookEventType

from .constants import INTERSECTION, UNION
from .feature_segments.serializers import (
Expand Down Expand Up @@ -460,7 +465,32 @@ def get_feature_state_value(self, obj):

def save(self, **kwargs):
try:
return super().save(**kwargs)
response = super().save(**kwargs)

feature_state = self.instance
if (
not feature_state.identity_id
and feature_state.feature.external_resources.exists()
and feature_state.environment.project.github_project.exists()
and feature_state.environment.project.organisation.github_config.exists()
):
github_configuration = GithubConfiguration.objects.get(
organisation_id=feature_state.environment.project.organisation_id
)
feature_states = []
feature_states.append(feature_state)
feature_data: GithubData = generate_data(
github_configuration=github_configuration,
feature=feature_state.feature,
type=WebhookEventType.FLAG_UPDATED.value,
feature_states=feature_states,
)

call_github_app_webhook_for_feature_state.delay(
args=(asdict(feature_data),),
)

return response
except django.core.exceptions.ValidationError as e:
raise serializers.ValidationError(str(e))

Expand Down
47 changes: 24 additions & 23 deletions api/integrations/github/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
import requests
from core.helpers import get_current_site_url
from django.conf import settings
from django.utils.formats import get_format

from features.models import FeatureState, FeatureStateValue
from features.models import Feature, FeatureState, FeatureStateValue
from integrations.github.client import generate_token
from integrations.github.constants import (
DELETED_FEATURE_TEXT,
Expand Down Expand Up @@ -89,26 +90,29 @@ def generate_body_comment(
if len(feature_states) > 0 and not feature_states[0].get("segment_name"):
result += FEATURE_TABLE_HEADER

for v in feature_states:
feature_value = v.get("feature_state_value")
tab = "segment-overrides" if v.get("segment_name") is not None else "value"
for fs in feature_states:
feature_value = fs.get("feature_state_value")
tab = "segment-overrides" if fs.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"),
fs.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"]
if (
fs.get("segment_name") is not None
and fs["segment_name"] != last_segment_name
):
Comment on lines +103 to +106
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming that last_segment_name is never None this should simplify to the below.

Suggested change
if (
fs.get("segment_name") is not None
and fs["segment_name"] != last_segment_name
):
if (
fs.get("segment_name") != last_segment_name
):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fs.get("segment_name") can be None

result += "\n" + LINK_SEGMENT_TITLE % (fs["segment_name"])
last_segment_name = fs["segment_name"]
result += FEATURE_TABLE_HEADER
table_row = FEATURE_TABLE_ROW % (
v["environment_name"],
fs["environment_name"],
environment_link_url,
"✅ Enabled" if v["enabled"] else "❌ Disabled",
"✅ Enabled" if fs["enabled"] else "❌ Disabled",
f"`{feature_value}`" if feature_value else "",
v["last_updated"],
fs["last_updated"],
)
result += table_row

Expand All @@ -121,31 +125,28 @@ def check_not_none(value: any) -> bool:

def generate_data(
github_configuration: GithubConfiguration,
feature_id: int,
feature_name: str,
feature: Feature,
type: str,
feature_states: (
typing.Union[list[FeatureState], list[FeatureStateValue]] | None
) = None,
url: str | None = None,
project_id: int | None = None,
) -> GithubData:

if feature_states:
feature_states_list = []
for feature_state in feature_states:
feature_state_value = feature_state.get_feature_state_value()
feature_state_value_type = feature_state.get_feature_state_value_type(
feature_state_value
)
feature_env_data = {}

if check_not_none(feature_state_value):
feature_env_data["feature_state_value"] = feature_state_value
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["enabled"] = feature_state.enabled
feature_env_data["last_updated"] = feature_state.updated_at
feature_env_data["last_updated"] = feature_state.updated_at.strftime(
get_format("DATETIME_INPUT_FORMATS")[0]
)
feature_env_data["environment_api_key"] = (
feature_state.environment.api_key
)
Expand All @@ -159,8 +160,8 @@ def generate_data(
feature_states_list.append(feature_env_data)

return GithubData(
feature_id=feature_id,
feature_name=feature_name,
feature_id=feature.id,
feature_name=feature.name,
installation_id=github_configuration.installation_id,
type=type,
url=(
Expand All @@ -169,5 +170,5 @@ def generate_data(
else None
),
feature_states=feature_states_list if feature_states else None,
project_id=project_id,
project_id=feature.project_id,
)
2 changes: 1 addition & 1 deletion api/integrations/github/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def send_post_request(data: CallGithubData) -> None:
feature_id = data.github_data.feature_id
project_id = data.github_data.project_id
event_type = data.event_type
feature_states = feature_states = (
feature_states = (
data.github_data.feature_states if data.github_data.feature_states else None
)
installation_id = data.github_data.installation_id
Expand Down
Loading