From e6de90994a83c1a341d365e4e89b74ffce755334 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Sun, 16 Aug 2026 13:17:44 +0100 Subject: [PATCH 1/2] refactor(Versioning): Publish a feature version with an AuthorData --- api/core/dataclasses.py | 4 ++ api/core/workflows_services.py | 5 +- api/features/future/services.py | 22 ++------- api/features/future/views.py | 5 +- api/features/versioning/models.py | 13 ++--- api/features/versioning/serializers.py | 21 ++------ api/features/versioning/tasks.py | 3 +- api/features/versioning/versioning_service.py | 12 ++--- api/features/versioning/views.py | 2 +- .../unit/audit/test_unit_audit_signals.py | 3 +- api/tests/unit/audit/test_unit_audit_views.py | 3 +- .../unit/core/test_unit_core_dataclasses.py | 11 +++++ .../test_unit_environments_views.py | 3 +- .../test_unit_feature_segments_views.py | 5 +- .../features/test_unit_features_models.py | 3 +- .../unit/features/test_unit_features_views.py | 15 +++--- .../versioning/test_unit_versioning_models.py | 11 +++-- .../versioning/test_unit_versioning_tasks.py | 11 +++-- ...test_unit_versioning_versioning_service.py | 16 +++--- .../versioning/test_unit_versioning_views.py | 49 +++++++++++++++---- .../util/mappers/test_unit_mappers_engine.py | 5 +- .../observability/_events-catalogue.md | 12 ++--- 22 files changed, 129 insertions(+), 105 deletions(-) create mode 100644 api/tests/unit/core/test_unit_core_dataclasses.py diff --git a/api/core/dataclasses.py b/api/core/dataclasses.py index 9769db6a8bc1..7d8058eee03f 100644 --- a/api/core/dataclasses.py +++ b/api/core/dataclasses.py @@ -13,6 +13,10 @@ class AuthorData: user: "FFAdminUser | None" = None api_key: "MasterAPIKey | None" = None + def __post_init__(self) -> None: + if self.user and self.api_key: + raise ValueError("Author must be either a user or a MasterAPIKey") + @classmethod def from_request(cls, request: "Request") -> "AuthorData": from users.models import FFAdminUser diff --git a/api/core/workflows_services.py b/api/core/workflows_services.py index 4481e08ed2ef..7a7ccf18dd61 100644 --- a/api/core/workflows_services.py +++ b/api/core/workflows_services.py @@ -4,6 +4,7 @@ from django.db import transaction from django.utils import timezone +from core.dataclasses import AuthorData from environments.tasks import rebuild_environment_document from features.versioning.models import EnvironmentFeatureVersion from features.versioning.signals import environment_feature_version_published @@ -84,7 +85,9 @@ def _publish_environment_feature_versions( ): environment_feature_version.live_from = now - environment_feature_version.publish(published_by, persist=False) + environment_feature_version.publish( + AuthorData(user=published_by), persist=False + ) EnvironmentFeatureVersion.objects.bulk_update( environment_feature_versions, diff --git a/api/features/future/services.py b/api/features/future/services.py index d2fe64aed62c..d18acdf1f17a 100644 --- a/api/features/future/services.py +++ b/api/features/future/services.py @@ -7,7 +7,7 @@ from django.db import transaction from django.db.models import Count, Q -from api_keys.user import APIKeyUser +from core.dataclasses import AuthorData from environments.models import Environment from features.future.exceptions import ( DuplicatePriorityError, @@ -29,7 +29,6 @@ from features.multivariate.models import MultivariateFeatureStateValue from features.versioning.models import EnvironmentFeatureVersion from features.versioning.versioning_service import get_environment_flags_list -from users.models import FFAdminUser logger = structlog.get_logger("features") @@ -73,17 +72,6 @@ def _get_feature_states_to_write( ) -def _publish_version( - version: EnvironmentFeatureVersion, author: FFAdminUser | APIKeyUser -) -> None: - # `UserABC.__subclasshook__` matches any user against `APIKeyUser` - published_by = author if isinstance(author, FFAdminUser) else None - version.publish( - published_by=published_by, - published_by_api_key=None if published_by else author.key, - ) - - def _get_overrides_by_segment_id( feature_states: Sequence[FeatureState], ) -> dict[int, FeatureState]: @@ -294,7 +282,7 @@ def update_flag( feature: Feature, changes: UpdateFlagRequest, replace: bool, - author: FFAdminUser | APIKeyUser, + author: AuthorData, ) -> UpdateFlagResponse: """Write the given parts of a flag, whichever versioning the environment uses.""" writes_nothing = not changes if replace else not any(changes.values()) @@ -329,7 +317,7 @@ def update_flag( ) if version is not None: - _publish_version(version, author) + version.publish(author) logger.info( "flag.updated", @@ -350,7 +338,7 @@ def delete_segment_override( environment: Environment, feature: Feature, segment_id: int, - author: FFAdminUser | APIKeyUser, + author: AuthorData, ) -> UpdateFlagResponse: """Remove a flag's override for one segment, leaving the rest of the flag alone.""" with transaction.atomic(): @@ -368,7 +356,7 @@ def delete_segment_override( ) if version is not None: - _publish_version(version, author) + version.publish(author) logger.info( "flag.updated", diff --git a/api/features/future/views.py b/api/features/future/views.py index a4d7b0cd047e..9af057f88a4e 100644 --- a/api/features/future/views.py +++ b/api/features/future/views.py @@ -9,6 +9,7 @@ from rest_framework.response import Response from rest_framework.views import APIView +from core.dataclasses import AuthorData from core.types import AuthenticatedRequest from environments.models import Environment from features.future.exceptions import ChangeRequestsEnabledError @@ -126,7 +127,7 @@ def _update_flag( feature=feature, changes=serializer.validated_data, replace=replace, - author=request.user, + author=AuthorData.from_request(request), ) ) @@ -162,6 +163,6 @@ def delete( environment=environment, feature=feature, segment_id=segment_id, - author=request.user, + author=AuthorData.from_request(request), ) ) diff --git a/api/features/versioning/models.py b/api/features/versioning/models.py index 93a85254bf69..a2f6d8718b27 100644 --- a/api/features/versioning/models.py +++ b/api/features/versioning/models.py @@ -14,7 +14,7 @@ ) from softdelete.models import SoftDeleteObject # type: ignore[import-untyped] -from api_keys.models import MasterAPIKey +from core.dataclasses import AuthorData from core.models import ( SoftDeleteExportableModel, abstract_base_auditable_model_factory, @@ -176,21 +176,18 @@ def get_previous_version(self) -> typing.Optional["EnvironmentFeatureVersion"]: def publish( self, - published_by: typing.Union["FFAdminUser", None] = None, - published_by_api_key: MasterAPIKey | None = None, + author: AuthorData | None = None, live_from: datetime.datetime | None = None, persist: bool = True, ) -> None: - assert not (published_by and published_by_api_key), ( - "Version must be published by either a user or a MasterAPIKey" - ) + author = author or AuthorData() now = timezone.now() self.live_from = live_from or (self.live_from or now) self.published_at = now - self.published_by = published_by - self.published_by_api_key = published_by_api_key + self.published_by = author.user + self.published_by_api_key = author.api_key if persist: self.save() diff --git a/api/features/versioning/serializers.py b/api/features/versioning/serializers.py index 0d75f8c5087c..3d96a8920973 100644 --- a/api/features/versioning/serializers.py +++ b/api/features/versioning/serializers.py @@ -4,7 +4,7 @@ from django.db import transaction from rest_framework import serializers -from api_keys.user import APIKeyUser +from core.dataclasses import AuthorData from environments.models import Environment from features.feature_segments.limits import ( SEGMENT_OVERRIDE_LIMIT_EXCEEDED_MESSAGE, @@ -20,7 +20,6 @@ post_gitlab_state_change_comment_for_feature_state, ) from segments.models import Segment -from users.models import FFAdminUser if typing.TYPE_CHECKING: from features.models import FeatureState @@ -203,11 +202,7 @@ def create( if self.validated_data.get("publish_immediately", False): request = self.context["request"] - version.publish( - published_by=( - request.user if isinstance(request.user, FFAdminUser) else None - ) - ) + version.publish(AuthorData.from_request(request)) return version # type: ignore[no-any-return] @@ -345,18 +340,8 @@ def save(self, **kwargs): # type: ignore[no-untyped-def] request = self.context["request"] - published_by = None - published_by_api_key = None - - if isinstance(request.user, FFAdminUser): - published_by = request.user - elif isinstance(request.user, APIKeyUser): - published_by_api_key = request.user.key - self.instance.publish( # type: ignore[union-attr] - live_from=live_from, - published_by=published_by, - published_by_api_key=published_by_api_key, + AuthorData.from_request(request), live_from=live_from ) return self.instance diff --git a/api/features/versioning/tasks.py b/api/features/versioning/tasks.py index abb885a88ad7..9dbb875df65b 100644 --- a/api/features/versioning/tasks.py +++ b/api/features/versioning/tasks.py @@ -12,6 +12,7 @@ from audit.constants import ENVIRONMENT_FEATURE_VERSION_PUBLISHED_MESSAGE from audit.models import AuditLog from audit.related_object_type import RelatedObjectType +from core.dataclasses import AuthorData from features.models import FeatureState from features.versioning.exceptions import FeatureVersioningError from features.versioning.models import ( @@ -345,7 +346,7 @@ def publish_version_change_set( # which might mean that actually version_change_set.live_from is slightly # in the past since the task processor won't have picked it up and handled # it immediately, but we always care about the _actual_ time it's published. - version.publish(published_by=user, live_from=now) + version.publish(AuthorData(user=user), live_from=now) # if live_from was set on the version_change set, then leave it alone for # auditing purposes. diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index f41fce484789..adf0c32f78a8 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -198,10 +198,7 @@ def _update_flag_for_versioning_v2( if change_set.segment_id is not None and change_set.segment_priority is not None: _update_segment_priority(target_feature_state, change_set.segment_priority) - new_version.publish( - published_by=change_set.author.user, - published_by_api_key=change_set.author.api_key, - ) + new_version.publish(change_set.author) return target_feature_state @@ -400,10 +397,7 @@ def _update_flag_v2_for_versioning_v2( ) update_multivariate_values(segment_state, override.multivariate_values) - new_version.publish( - published_by=change_set.author.user, - published_by_api_key=change_set.author.api_key, - ) + new_version.publish(change_set.author) def _update_flag_v2_for_versioning_v1( @@ -523,7 +517,7 @@ def _delete_segment_override_v2( ) segment_feature_state.feature_segment.delete() - new_version.publish(published_by=author.user, published_by_api_key=author.api_key) + new_version.publish(author) def get_updated_feature_states_for_version( diff --git a/api/features/versioning/views.py b/api/features/versioning/views.py index 7245bf1ac2ca..26fd83ceeb3e 100644 --- a/api/features/versioning/views.py +++ b/api/features/versioning/views.py @@ -160,7 +160,7 @@ def publish(self, request: Request, **kwargs) -> Response: # type: ignore[no-un ef_version = self.get_object() serializer = self.get_serializer(data=request.data, instance=ef_version) serializer.is_valid(raise_exception=True) - serializer.save(published_by=request.user) + serializer.save() return Response(serializer.data) def _apply_visibility_limits(self, queryset: QuerySet) -> QuerySet: # type: ignore[type-arg] diff --git a/api/tests/unit/audit/test_unit_audit_signals.py b/api/tests/unit/audit/test_unit_audit_signals.py index 5ddf4e671396..21546184aa42 100644 --- a/api/tests/unit/audit/test_unit_audit_signals.py +++ b/api/tests/unit/audit/test_unit_audit_signals.py @@ -15,6 +15,7 @@ send_feature_flag_went_live_signal, trigger_feature_state_change_webhooks, ) +from core.dataclasses import AuthorData from environments.models import Environment from features.models import Feature, FeatureState from features.versioning.models import EnvironmentFeatureVersion @@ -345,7 +346,7 @@ def _create_and_publish_environment_feature_version( environment=environment, feature=feature, ) - version.publish(user) + version.publish(AuthorData(user=user)) audit_log_record = ( AuditLog.objects.filter( diff --git a/api/tests/unit/audit/test_unit_audit_views.py b/api/tests/unit/audit/test_unit_audit_views.py index 403181420375..35fcedea5404 100644 --- a/api/tests/unit/audit/test_unit_audit_views.py +++ b/api/tests/unit/audit/test_unit_audit_views.py @@ -12,6 +12,7 @@ from audit.constants import ENVIRONMENT_FEATURE_VERSION_PUBLISHED_MESSAGE from audit.models import AuditLog from audit.related_object_type import RelatedObjectType +from core.dataclasses import AuthorData from environments.models import Environment from features.models import Feature from features.versioning.models import EnvironmentFeatureVersion @@ -182,7 +183,7 @@ def test_retrieve_audit_log__environment_feature_version_published__includes_req feature=feature, environment=environment_v2_versioning, ) - new_version.publish(published_by=admin_user) + new_version.publish(AuthorData(user=admin_user)) audit_log = ( AuditLog.objects.filter(related_object_type=RelatedObjectType.EF_VERSION.name) diff --git a/api/tests/unit/core/test_unit_core_dataclasses.py b/api/tests/unit/core/test_unit_core_dataclasses.py new file mode 100644 index 000000000000..f008fc063a78 --- /dev/null +++ b/api/tests/unit/core/test_unit_core_dataclasses.py @@ -0,0 +1,11 @@ +import pytest + +from api_keys.models import MasterAPIKey +from core.dataclasses import AuthorData +from users.models import FFAdminUser + + +def test_author_data__user_and_api_key__raises_value_error() -> None: + # Given / When / Then + with pytest.raises(ValueError): + AuthorData(user=FFAdminUser(), api_key=MasterAPIKey()) diff --git a/api/tests/unit/environments/test_unit_environments_views.py b/api/tests/unit/environments/test_unit_environments_views.py index 1b53ada87a55..2742a1990e2f 100644 --- a/api/tests/unit/environments/test_unit_environments_views.py +++ b/api/tests/unit/environments/test_unit_environments_views.py @@ -18,6 +18,7 @@ from api_keys.models import MasterAPIKey from audit.models import AuditLog, RelatedObjectType # type: ignore[attr-defined] from core.constants import STRING +from core.dataclasses import AuthorData from environments.identities.models import Identity from environments.identities.traits.models import Trait from environments.models import Environment, EnvironmentAPIKey, Webhook @@ -1359,7 +1360,7 @@ def test_retrieve_environment__v2_versioning_with_old_versions__returns_correct_ EnvironmentFeatureVersion.objects.create( feature=feature, environment=environment_v2_versioning - ).publish(staff_user) + ).publish(AuthorData(user=staff_user)) # When response = admin_client_new.get(url) diff --git a/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py b/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py index afd53083416a..c61fe1ae516d 100644 --- a/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py +++ b/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py @@ -17,6 +17,7 @@ from audit.constants import SEGMENT_FEATURE_STATE_DELETED_MESSAGE from audit.models import AuditLog from audit.related_object_type import RelatedObjectType +from core.dataclasses import AuthorData from environments.models import Environment from features.models import Feature, FeatureSegment, FeatureState from features.versioning.models import EnvironmentFeatureVersion @@ -647,14 +648,14 @@ def test_get_feature_segments__v2_versioning__returns_only_latest_version( feature_segment=feature_segment_v1, environment_feature_version=version_1, ) - version_1.publish(staff_user, persist=True) + version_1.publish(AuthorData(user=staff_user), persist=True) # and let's create another new version, which will trigger a duplication # of the feature segment into the new version version_2 = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - version_2.publish(published_by=staff_user, persist=True) + version_2.publish(AuthorData(user=staff_user), persist=True) # Let's grab the latest versioned feature segment, so we can check for it's # (exclusive) existence in the response from the API. diff --git a/api/tests/unit/features/test_unit_features_models.py b/api/tests/unit/features/test_unit_features_models.py index f81be84a3f97..42fdb83279d5 100644 --- a/api/tests/unit/features/test_unit_features_models.py +++ b/api/tests/unit/features/test_unit_features_models.py @@ -9,6 +9,7 @@ from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped] from pytest_mock import MockerFixture +from core.dataclasses import AuthorData from environments.identities.models import Identity from environments.models import Environment from features.constants import ENVIRONMENT, FEATURE_SEGMENT, IDENTITY @@ -1476,7 +1477,7 @@ def test_feature_state_create__with_environment_feature_version__does_not_trigge environment=environment_v2_versioning, feature_segment=feature_segment, ) - new_version.publish(admin_user) + new_version.publish(AuthorData(user=admin_user)) # Then - Webhooks are not triggered for versioned environments # (handled by trigger_update_version_webhooks instead) diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index cf7554904ed5..a99f1b51a3cb 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -36,6 +36,7 @@ ) from audit.models import AuditLog, RelatedObjectType # type: ignore[attr-defined] from core.constants import FLAGSMITH_UPDATED_AT_HEADER, SDK_ENVIRONMENT_KEY_HEADER +from core.dataclasses import AuthorData from environments.dynamodb import ( DynamoEnvironmentV2Wrapper, DynamoIdentityWrapper, @@ -2536,7 +2537,7 @@ def test_list_features__v2_versioning_with_removed_override__returns_zero_overri ), environment_feature_version=version_2, ) - version_2.publish(admin_user) + version_2.publish(AuthorData(user=admin_user)) # and now let's create a new version which removes the segment override version_3 = EnvironmentFeatureVersion.objects.create( @@ -2546,7 +2547,7 @@ def test_list_features__v2_versioning_with_removed_override__returns_zero_overri environment_feature_version=version_3, feature_segment__segment=segment, ).delete() - version_3.publish(admin_user) + version_3.publish(AuthorData(user=admin_user)) # When response = admin_client_new.get(url) @@ -4023,9 +4024,9 @@ def test_list_features__value_search_string_and_int__returns_matching( feature_state1b.enabled = False feature_state1b.save() - environment_feature_version1b.publish(staff_user) + environment_feature_version1b.publish(AuthorData(user=staff_user)) - environment_feature_version1.publish(staff_user) + environment_feature_version1.publish(AuthorData(user=staff_user)) feature_state_value1 = feature_state1.feature_state_value feature_state_value1.set_value("1945", "integer") @@ -4295,12 +4296,12 @@ def test_list_feature_states__v2_versioning__returns_latest_versions( environment=environment_v2_versioning, environment_feature_version=version_2, ) - version_2.publish(staff_user) + version_2.publish(AuthorData(user=staff_user)) version_3 = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - version_3.publish(staff_user) + version_3.publish(AuthorData(user=staff_user)) # When response = staff_client.get(url) @@ -4398,7 +4399,7 @@ def _assert_feature_list_last_modified_values( # type: ignore[no-untyped-def] environment=environment_v2_versioning_2, feature=feature ) ) - environment_v2_versioning_2_version_2.publish(staff_user) + environment_v2_versioning_2_version_2.publish(AuthorData(user=staff_user)) with freeze_time(now): # and create a new unpublished version in the current environment, simulated to be now diff --git a/api/tests/unit/features/versioning/test_unit_versioning_models.py b/api/tests/unit/features/versioning/test_unit_versioning_models.py index 459d2787e0f2..727613f82fd2 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_models.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_models.py @@ -7,6 +7,7 @@ from pytest_mock import MockerFixture from core.constants import STRING +from core.dataclasses import AuthorData from environments.models import Environment from environments.tasks import rebuild_environment_document from features.models import Feature, FeatureSegment, FeatureState @@ -184,11 +185,11 @@ def test_get_previous_version__more_recent_version_published__returns_correct_pr version_1 = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - version_1.publish(admin_user) + version_1.publish(AuthorData(user=admin_user)) version_2 = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - version_2.publish(admin_user) + version_2.publish(AuthorData(user=admin_user)) # When previous_version = version_1.get_previous_version() @@ -220,7 +221,7 @@ def test_environment_feature_version_publish__valid_version__sets_live_and_trigg # When with freeze_time(now): - version_2.publish(published_by=admin_user) + version_2.publish(AuthorData(user=admin_user)) # Then assert version_2.is_live @@ -251,7 +252,7 @@ def test_environment_feature_version_publish__new_version__triggers_update_webho ) # When - new_version.publish(admin_user) + new_version.publish(AuthorData(user=admin_user)) # Then mock_trigger_update_version_webhooks.delay.assert_called_once_with( @@ -276,7 +277,7 @@ def test_get_latest_versions__future_scheduled_version__excludes_from_results( feature=feature, live_from=timezone.now() + timedelta(hours=1), ) - scheduled_version.publish(admin_user) + scheduled_version.publish(AuthorData(user=admin_user)) # When latest_versions = EnvironmentFeatureVersion.objects.get_latest_versions_as_queryset( diff --git a/api/tests/unit/features/versioning/test_unit_versioning_tasks.py b/api/tests/unit/features/versioning/test_unit_versioning_tasks.py index d34656841eae..336b406619b5 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_tasks.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_tasks.py @@ -13,6 +13,7 @@ from rest_framework.exceptions import ValidationError from core.constants import STRING +from core.dataclasses import AuthorData from environments.identities.models import Identity from environments.models import Environment, Webhook from features.models import Feature, FeatureSegment, FeatureState @@ -88,7 +89,7 @@ def test_disable_v2_versioning__published_and_unpublished_versions__restores_lat environment_feature_version=v2, ) - v2.publish(staff_user) + v2.publish(AuthorData(user=staff_user)) # Now, let's create a new version which we won't publish (and hence should be ignored after we disabled # v2 versioning) @@ -174,7 +175,7 @@ def test_trigger_update_version_webhooks__version_with_changes__triggers_flag_up v2_fs = v2.feature_states.first() v2_fs.enabled = not v1_fs.enabled # Make a change v2_fs.save() - v2.publish(published_by=staff_user) + v2.publish(AuthorData(user=staff_user)) # Setup webhooks from organisations.models import OrganisationWebhook @@ -262,12 +263,12 @@ def test_trigger_update_version_webhooks__version_without_changes__triggers_only v1 = EnvironmentFeatureVersion.objects.get( feature=feature, environment=environment_v2_versioning ) - v1.publish(published_by=staff_user) + v1.publish(AuthorData(user=staff_user)) v2 = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - v2.publish(published_by=staff_user) + v2.publish(AuthorData(user=staff_user)) # Setup webhook environment_webhook_url = "https://example.com/env-webhook/" @@ -329,7 +330,7 @@ def test_trigger_update_version_webhooks__multivariate_feature__includes_mv_valu original_allocation = bumped_mv_value.percentage_allocation bumped_mv_value.percentage_allocation = original_allocation + 5 bumped_mv_value.save() - v2.publish(published_by=staff_user) + v2.publish(AuthorData(user=staff_user)) environment_webhook_url = "https://example.com/env-webhook/" Webhook.objects.create( diff --git a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py index f190e7d86741..c5b78f84bc3e 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_versioning_service.py @@ -271,7 +271,7 @@ def test_get_environment_flags_list__v2_versioning_with_published_version__retur ) environment_feature_1_version_2_feature_state.enabled = True environment_feature_1_version_2_feature_state.save() - environment_feature_1_version_2.publish(admin_user) + environment_feature_1_version_2.publish(AuthorData(user=admin_user)) # When with django_assert_num_queries(2): @@ -317,7 +317,7 @@ def test_get_environment_flags_list__v2_segment_override_removed__excludes_overr feature_segment__segment=segment, environment_feature_version=new_version, ).delete() - new_version.publish(published_by=admin_user) + new_version.publish(AuthorData(user=admin_user)) # When environment_feature_states = get_environment_flags_list( @@ -346,7 +346,9 @@ def test_get_current_live_environment_feature_version__unpublished_and_future_ve future_version = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - future_version.publish(staff_user, live_from=timezone.now() + timedelta(days=1)) + future_version.publish( + AuthorData(user=staff_user), live_from=timezone.now() + timedelta(days=1) + ) # When latest_version = get_current_live_environment_feature_version( @@ -495,7 +497,7 @@ def test_get_updated_feature_states_for_version__environment_value_changed__retu v1_segment_override.feature_state_value.type = STRING v1_segment_override.feature_state_value.string_value = "segment_value" v1_segment_override.feature_state_value.save() - v1.publish(published_by=staff_user) + v1.publish(AuthorData(user=staff_user)) # Create v2 - environment feature state value changes but segment override stays same v2 = EnvironmentFeatureVersion.objects.create( @@ -544,7 +546,7 @@ def test_get_updated_feature_states_for_version__segment_override_value_changed_ v1_segment_override.feature_state_value.type = STRING v1_segment_override.feature_state_value.string_value = "segment_value_v1" v1_segment_override.feature_state_value.save() - v1.publish(published_by=staff_user) + v1.publish(AuthorData(user=staff_user)) # Create v2 - segment override value changes but environment default stays same v2 = EnvironmentFeatureVersion.objects.create( @@ -618,7 +620,7 @@ def test_get_updated_feature_states_for_version__mv_allocation_changed__returns_ multivariate_feature_option=mv_option_2, percentage_allocation=40, ) - v1.publish(published_by=staff_user) + v1.publish(AuthorData(user=staff_user)) # Create v2 - change the multivariate percentage allocations in segment override v2 = EnvironmentFeatureVersion.objects.create( @@ -700,7 +702,7 @@ def test_get_updated_feature_states_for_version__mv_control_value_changed__retur multivariate_feature_option=mv_option_2, percentage_allocation=50, ) - v1.publish(published_by=staff_user) + v1.publish(AuthorData(user=staff_user)) # Create v2 - change the control value but keep multivariate allocations the same v2 = EnvironmentFeatureVersion.objects.create( diff --git a/api/tests/unit/features/versioning/test_unit_versioning_views.py b/api/tests/unit/features/versioning/test_unit_versioning_views.py index 3b633ad52032..537bbe32bc8d 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_views.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_views.py @@ -22,6 +22,7 @@ from audit.models import AuditLog from audit.related_object_type import RelatedObjectType from core.constants import STRING +from core.dataclasses import AuthorData from environments.models import Environment from features.feature_segments.limits import ( SEGMENT_OVERRIDE_LIMIT_EXCEEDED_MESSAGE, @@ -68,7 +69,7 @@ def test_list_versions__v2_versioning_enabled__returns_all_versions( version_2 = EnvironmentFeatureVersion.objects.create( feature=feature, environment=environment_v2_versioning ) - version_2.publish(published_by=admin_user) + version_2.publish(AuthorData(user=admin_user)) # and a draft version draft_version = EnvironmentFeatureVersion.objects.create( @@ -119,6 +120,34 @@ def test_create_feature_version__staff_with_permissions__returns_created( assert response_json["uuid"] +def test_create_feature_version__master_api_key__publishes_with_api_key_attribution( + admin_master_api_key: MasterAPIKey, + admin_master_api_key_client: APIClient, + environment_v2_versioning: Environment, + feature: Feature, +) -> None: + # Given + url = reverse( + "api-v1:versioning:environment-feature-versions-list", + args=[environment_v2_versioning.id, feature.id], + ) + + # When + response = admin_master_api_key_client.post( + url, + data=json.dumps({"publish_immediately": True}), + content_type="application/json", + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + version = EnvironmentFeatureVersion.objects.get(uuid=response.json()["uuid"]) + assert version.published is True + assert version.published_by is None + assert version.published_by_api_key == admin_master_api_key[0] + + def test_delete_feature_version__unpublished_version__marks_as_deleted( admin_client: APIClient, environment_v2_versioning: Environment, @@ -216,7 +245,7 @@ def test_retrieve_feature_version__has_previous_version__returns_previous_uuid( version_2 = EnvironmentFeatureVersion.objects.create( feature=feature, environment=environment_v2_versioning ) - version_2.publish(published_by=staff_user) + version_2.publish(AuthorData(user=staff_user)) url = reverse("api-v1:versioning:get-efv-by-uuid", args=[version_2.uuid]) @@ -642,7 +671,7 @@ def test_delete_feature_version_feature_state__published_version__returns_bad_re ) # and we publish the version - environment_feature_version.publish(admin_user) + environment_feature_version.publish(AuthorData(user=admin_user)) url = reverse( "api-v1:versioning:environment-feature-version-featurestates-detail", @@ -732,7 +761,7 @@ def test_list_versions__filter_by_is_live__returns_matching_versions( published_environment_feature_version = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - published_environment_feature_version.publish(staff_user) + published_environment_feature_version.publish(AuthorData(user=staff_user)) _base_url = reverse( "api-v1:versioning:environment-feature-versions-list", @@ -1620,7 +1649,7 @@ def test_list_versions__non_enterprise_plan__returns_only_recent_versions( version = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - version.publish(staff_user) + version.publish(AuthorData(user=staff_user)) outside_limit_versions.append(version) # Now let's jump to the current time and create some versions which @@ -1632,7 +1661,7 @@ def test_list_versions__non_enterprise_plan__returns_only_recent_versions( version = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - version.publish(staff_user) + version.publish(AuthorData(user=staff_user)) inside_limit_versions.append(version) # When @@ -1676,7 +1705,7 @@ def test_list_versions__current_version_outside_limit__still_returns_current( latest_version = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - latest_version.publish(staff_user) + latest_version.publish(AuthorData(user=staff_user)) # When # we jump to the current time and retrieve the versions @@ -1737,7 +1766,7 @@ def test_list_versions__enterprise_plan_saas__returns_all_versions( version = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - version.publish(staff_user) + version.publish(AuthorData(user=staff_user)) all_versions.append(version) # Now let's jump to the current time and create some versions which @@ -1748,7 +1777,7 @@ def test_list_versions__enterprise_plan_saas__returns_all_versions( version = EnvironmentFeatureVersion.objects.create( environment=environment_v2_versioning, feature=feature ) - version.publish(staff_user) + version.publish(AuthorData(user=staff_user)) all_versions.append(version) # When @@ -1822,7 +1851,7 @@ def test_create_feature_version_feature_state__override_recreated_in_draft__inhe environment_feature_version=overridden_version, ), ) - overridden_version.publish(published_by=admin_user) + overridden_version.publish(AuthorData(user=admin_user)) # and a draft version from which the (cloned) override has been removed draft_version = EnvironmentFeatureVersion.objects.create( diff --git a/api/tests/unit/util/mappers/test_unit_mappers_engine.py b/api/tests/unit/util/mappers/test_unit_mappers_engine.py index f941ad82abad..783f08bd87c1 100644 --- a/api/tests/unit/util/mappers/test_unit_mappers_engine.py +++ b/api/tests/unit/util/mappers/test_unit_mappers_engine.py @@ -6,6 +6,7 @@ from django.utils import timezone from pytest_mock import MockerFixture +from core.dataclasses import AuthorData from environments.identities.models import Identity from environments.identities.traits.models import Trait from environments.models import Environment @@ -772,7 +773,7 @@ def test_map_environment_to_engine__v2_versioning_segment_override_removed__retu environment_feature_version=v2, ), ) - v2.publish(staff_user) + v2.publish(AuthorData(user=staff_user)) # Now, let's create another new version which will keep one of the segment overrides # and remove the other. @@ -787,7 +788,7 @@ def test_map_environment_to_engine__v2_versioning_segment_override_removed__retu feature_segment__segment=another_segment, environment_feature_version=v3 ).delete() - v3.publish(staff_user) + v3.publish(AuthorData(user=staff_user)) # When environment_model = engine.map_environment_to_engine(environment_v2_versioning) diff --git a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md index 6853285861c7..59d3b4ec5d0b 100644 --- a/docs/docs/deployment-self-hosting/observability/_events-catalogue.md +++ b/docs/docs/deployment-self-hosting/observability/_events-catalogue.md @@ -267,7 +267,7 @@ Attributes: ### `features.flag.update_rejected` Logged at `warning` from: - - `api/features/future/views.py:49` + - `api/features/future/views.py:50` Attributes: - `environment.id` @@ -279,8 +279,8 @@ Attributes: ### `features.flag.updated` Logged at `info` from: - - `api/features/future/services.py:334` - - `api/features/future/services.py:373` + - `api/features/future/services.py:322` + - `api/features/future/services.py:361` Attributes: - `environment.id` @@ -906,7 +906,7 @@ Attributes: ### `workflows.change_request.committed` Logged at `info` from: - - `api/core/workflows_services.py:45` + - `api/core/workflows_services.py:46` Attributes: - `environment.id` @@ -916,7 +916,7 @@ Attributes: ### `workflows.missing_live_segment` Logged at `warning` from: - - `api/core/workflows_services.py:130` + - `api/core/workflows_services.py:133` Attributes: - `draft_segment` @@ -924,7 +924,7 @@ Attributes: ### `workflows.segment_revision_created` Logged at `info` from: - - `api/core/workflows_services.py:135` + - `api/core/workflows_services.py:138` Attributes: - `revision_id` From e831228e9e39a5fa78a1ce39fcfd288ed1e02f89 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Sun, 16 Aug 2026 13:32:16 +0100 Subject: [PATCH 2/2] test(Versioning): Split the GWT comment the linter cannot read --- api/tests/unit/core/test_unit_core_dataclasses.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/api/tests/unit/core/test_unit_core_dataclasses.py b/api/tests/unit/core/test_unit_core_dataclasses.py index f008fc063a78..20970c903313 100644 --- a/api/tests/unit/core/test_unit_core_dataclasses.py +++ b/api/tests/unit/core/test_unit_core_dataclasses.py @@ -6,6 +6,10 @@ def test_author_data__user_and_api_key__raises_value_error() -> None: - # Given / When / Then + # Given + user = FFAdminUser() + api_key = MasterAPIKey() + + # When / Then with pytest.raises(ValueError): - AuthorData(user=FFAdminUser(), api_key=MasterAPIKey()) + AuthorData(user=user, api_key=api_key)