refactor(Versioning): Publish a feature version with an AuthorData

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

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,

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",

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),
             )
         )

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()

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
 

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.

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(

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]

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(

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)

api/tests/unit/core/test_unit_core_dataclasses.py

@@ -0,0 +1,15 @@
+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
+    user = FFAdminUser()
+    api_key = MasterAPIKey()
+
+    # When / Then
+    with pytest.raises(ValueError):
+        AuthorData(user=user, api_key=api_key)

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)

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.

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)