diff --git a/api/conftest.py b/api/conftest.py index a54679405478..ab0aa198e9e3 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -336,6 +336,11 @@ def segment(project: Project): return _segment +@pytest.fixture() +def another_segment(project: Project) -> Segment: + return Segment.objects.create(name="another_segment", project=project) + + @pytest.fixture() def segment_rule(segment): return SegmentRule.objects.create(segment=segment, type=SegmentRule.ALL_RULE) @@ -587,6 +592,19 @@ def segment_featurestate(feature_segment, feature, environment): ) +@pytest.fixture() +def another_segment_featurestate( + feature: Feature, environment: Environment, another_segment: Segment +) -> FeatureState: + return FeatureState.objects.create( + feature_segment=FeatureSegment.objects.create( + feature=feature, segment=another_segment, environment=environment + ), + feature=feature, + environment=environment, + ) + + @pytest.fixture() def feature_with_value_segment( feature_with_value: Feature, segment: Segment, environment: Environment diff --git a/api/features/feature_segments/serializers.py b/api/features/feature_segments/serializers.py index ff0b58ced4a8..a4a2739cd304 100644 --- a/api/features/feature_segments/serializers.py +++ b/api/features/feature_segments/serializers.py @@ -1,5 +1,8 @@ import typing +from common.features.serializers import ( + CreateSegmentOverrideFeatureSegmentSerializer, +) from rest_framework import serializers from rest_framework.exceptions import PermissionDenied @@ -38,16 +41,14 @@ def validate(self, data): return data -class CreateSegmentOverrideFeatureSegmentSerializer(serializers.ModelSerializer): +class CustomCreateSegmentOverrideFeatureSegmentSerializer( + CreateSegmentOverrideFeatureSegmentSerializer +): # Since the `priority` field on the FeatureSegment model is set to editable=False # (to adhere to the django-ordered-model functionality), we redefine the priority # field here, and use it manually in the save method. priority = serializers.IntegerField(min_value=0, required=False) - class Meta: - model = FeatureSegment - fields = ("id", "segment", "priority", "uuid") - def save(self, **kwargs: typing.Any) -> FeatureSegment: priority: int | None = self.initial_data.pop("priority", None) diff --git a/api/features/multivariate/serializers.py b/api/features/multivariate/serializers.py index 524e51d6ebc6..69423fd70cf2 100644 --- a/api/features/multivariate/serializers.py +++ b/api/features/multivariate/serializers.py @@ -3,10 +3,7 @@ from rest_framework import serializers from features.models import Feature -from features.multivariate.models import ( - MultivariateFeatureOption, - MultivariateFeatureStateValue, -) +from features.multivariate.models import MultivariateFeatureOption class NestedMultivariateFeatureOptionSerializer(serializers.ModelSerializer): @@ -54,13 +51,3 @@ def _get_siblings(self, feature: Feature): siblings = siblings.exclude(id=self.instance.id) return siblings - - -class MultivariateFeatureStateValueSerializer(serializers.ModelSerializer): - class Meta: - model = MultivariateFeatureStateValue - fields = ( - "id", - "multivariate_feature_option", - "percentage_allocation", - ) diff --git a/api/features/serializers.py b/api/features/serializers.py index d98c4b70a14e..a2a297238637 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -2,6 +2,13 @@ from datetime import datetime import django.core.exceptions +from common.features.multivariate.serializers import ( + MultivariateFeatureStateValueSerializer, +) +from common.features.serializers import ( + CreateSegmentOverrideFeatureStateSerializer, + FeatureStateValueSerializer, +) from drf_writable_nested import WritableNestedModelSerializer from drf_yasg.utils import swagger_serializer_method from rest_framework import serializers @@ -27,13 +34,10 @@ from .constants import INTERSECTION, UNION from .feature_segments.serializers import ( - CreateSegmentOverrideFeatureSegmentSerializer, -) -from .models import Feature, FeatureState, FeatureStateValue -from .multivariate.serializers import ( - MultivariateFeatureStateValueSerializer, - NestedMultivariateFeatureOptionSerializer, + CustomCreateSegmentOverrideFeatureSegmentSerializer, ) +from .models import Feature, FeatureState +from .multivariate.serializers import NestedMultivariateFeatureOptionSerializer class FeatureStateSerializerSmall(serializers.ModelSerializer): @@ -551,12 +555,6 @@ class Meta: fields = ("feature", "enabled") -class FeatureStateValueSerializer(serializers.ModelSerializer): - class Meta: - model = FeatureStateValue - fields = ("type", "string_value", "integer_value", "boolean_value") - - class FeatureInfluxDataSerializer(serializers.Serializer): events_list = serializers.ListSerializer(child=serializers.DictField()) @@ -598,46 +596,12 @@ class SDKFeatureStatesQuerySerializer(serializers.Serializer): ) -class CreateSegmentOverrideFeatureStateSerializer(WritableNestedModelSerializer): - feature_state_value = FeatureStateValueSerializer() - feature_segment = CreateSegmentOverrideFeatureSegmentSerializer( +class CustomCreateSegmentOverrideFeatureStateSerializer( + CreateSegmentOverrideFeatureStateSerializer +): + feature_segment = CustomCreateSegmentOverrideFeatureSegmentSerializer( required=False, allow_null=True ) - multivariate_feature_state_values = MultivariateFeatureStateValueSerializer( - many=True, required=False - ) - - class Meta: - model = FeatureState - fields = ( - "id", - "feature", - "enabled", - "feature_state_value", - "feature_segment", - "deleted_at", - "uuid", - "created_at", - "updated_at", - "live_from", - "environment", - "identity", - "change_request", - "multivariate_feature_state_values", - ) - - read_only_fields = ( - "id", - "deleted_at", - "uuid", - "created_at", - "updated_at", - "live_from", - "environment", - "identity", - "change_request", - "feature", - ) def _get_save_kwargs(self, field_name): kwargs = super()._get_save_kwargs(field_name) diff --git a/api/features/versioning/dataclasses.py b/api/features/versioning/dataclasses.py new file mode 100644 index 000000000000..506ef7c861b0 --- /dev/null +++ b/api/features/versioning/dataclasses.py @@ -0,0 +1,14 @@ +from datetime import datetime + +from pydantic import BaseModel, computed_field + + +class Conflict(BaseModel): + segment_id: int | None = None + original_cr_id: int | None = None + published_at: datetime | None = None + + @computed_field + @property + def is_environment_default(self) -> bool: + return self.segment_id is None diff --git a/api/features/versioning/migrations/0004_add_version_change_set.py b/api/features/versioning/migrations/0004_add_version_change_set.py new file mode 100644 index 000000000000..ccc90069f686 --- /dev/null +++ b/api/features/versioning/migrations/0004_add_version_change_set.py @@ -0,0 +1,44 @@ +# Generated by Django 3.2.25 on 2024-07-10 11:18 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django_lifecycle.mixins + + +class Migration(migrations.Migration): + + dependencies = [ + ('environments', '0034_alter_environment_project'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('workflows_core', '0009_prevent_cascade_delete_from_user_delete'), + ('features', '0064_fix_feature_help_text_typo'), + ('feature_versioning', '0003_cascade_delete_versions_on_cr_delete'), + ] + + operations = [ + migrations.CreateModel( + name='VersionChangeSet', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('deleted_at', models.DateTimeField(blank=True, db_index=True, default=None, editable=False, null=True)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('published_at', models.DateTimeField(blank=True, null=True)), + ('live_from', models.DateTimeField(null=True)), + ('feature_states_to_create', models.TextField(help_text='JSON blob describing the feature states that should be created when the change request is published', null=True)), + ('feature_states_to_update', models.TextField(help_text='JSON blob describing the feature states that should be updated when the change request is published', null=True)), + ('segment_ids_to_delete_overrides', models.TextField(help_text='JSON blob describing the segment overrides for whichthe segment overrides should be deleted when the change request is published', null=True)), + ('change_request', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='change_sets', to='workflows_core.changerequest')), + ('environment', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='environments.environment')), + ('environment_feature_version', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='feature_versioning.environmentfeatureversion')), + ('feature', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='features.feature')), + ('published_by', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'permissions': (('can_undelete', 'Can undelete this object'),), + 'abstract': False, + }, + bases=(django_lifecycle.mixins.LifecycleModelMixin, models.Model), + ), + ] diff --git a/api/features/versioning/models.py b/api/features/versioning/models.py index 91234ce387e9..841a195c6720 100644 --- a/api/features/versioning/models.py +++ b/api/features/versioning/models.py @@ -1,4 +1,5 @@ import datetime +import json import typing import uuid from copy import deepcopy @@ -11,8 +12,11 @@ from django.db import models from django.db.models import Index from django.utils import timezone +from django_lifecycle import BEFORE_CREATE, LifecycleModelMixin, hook +from softdelete.models import SoftDeleteObject from api_keys.models import MasterAPIKey +from features.versioning.dataclasses import Conflict from features.versioning.exceptions import FeatureVersioningError from features.versioning.managers import EnvironmentFeatureVersionManager from features.versioning.signals import environment_feature_version_published @@ -163,3 +167,210 @@ def clone_to_environment( _clone.save() return _clone + + +class VersionChangeSet(LifecycleModelMixin, SoftDeleteObject): + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) + published_at = models.DateTimeField(blank=True, null=True) + published_by = models.ForeignKey( + "users.FFAdminUser", + blank=True, + null=True, + on_delete=models.SET_NULL, + ) + + environment = models.ForeignKey( + "environments.Environment", on_delete=models.CASCADE + ) + + change_request = models.ForeignKey( + "workflows_core.ChangeRequest", + on_delete=models.CASCADE, + related_name="change_sets", + blank=True, + null=True, + ) + environment_feature_version = models.ForeignKey( + "feature_versioning.EnvironmentFeatureVersion", + on_delete=models.CASCADE, + # Needs to be blank/nullable since change sets + # associated with a change request do not yet + # have a version + blank=True, + null=True, + ) + + feature = models.ForeignKey( + "features.Feature", + on_delete=models.CASCADE, + ) + live_from = models.DateTimeField(null=True) + + feature_states_to_create = models.TextField( + null=True, + help_text="JSON blob describing the feature states that should be " + "created when the change request is published", + ) + feature_states_to_update = models.TextField( + null=True, + help_text="JSON blob describing the feature states that should be " + "updated when the change request is published", + ) + segment_ids_to_delete_overrides = models.TextField( + null=True, + help_text="JSON blob describing the segment overrides for which" + "the segment overrides should be deleted when the change " + "request is published", + ) + + @hook(BEFORE_CREATE) + def add_environment(self): + if not self.environment_id: + if self.change_request_id: + self.environment = self.change_request.environment + elif self.environment_feature_version_id: + self.environment = self.environment_feature_version.environment + else: + raise RuntimeError( + "Version change set should belong to either a change request, or a version." + ) + + def get_parsed_feature_states_to_create(self) -> list[dict[str, typing.Any]]: + if self.feature_states_to_create: + return json.loads(self.feature_states_to_create) + return [] + + def get_parsed_feature_states_to_update(self) -> list[dict[str, typing.Any]]: + if self.feature_states_to_update: + return json.loads(self.feature_states_to_update) + return [] + + def get_parsed_segment_ids_to_delete_overrides(self) -> list[int]: + if self.segment_ids_to_delete_overrides: + return json.loads(self.segment_ids_to_delete_overrides) + return [] + + def publish(self, user: "FFAdminUser") -> None: + from features.versioning.tasks import publish_version_change_set + + kwargs = {"version_change_set_id": self.id, "user_id": user.id} + if not self.live_from or self.live_from < timezone.now(): + publish_version_change_set(**kwargs) + else: + kwargs["is_scheduled"] = True + publish_version_change_set.delay(kwargs=kwargs, delay_until=self.live_from) + + def get_conflicts(self) -> list[Conflict]: + if self.published_at: + return [] + + change_sets_since_creation = list( + self.__class__.objects.filter( + published_at__gte=self.created_at, + feature=self.feature, + environment=self.environment, + ).select_related("change_request") + ) + if not change_sets_since_creation: + return [] + + conflicts = [ + *self._get_conflicts_in_feature_states_to_update( + change_sets_since_creation + ), + *self._get_conflicts_in_feature_states_to_create( + change_sets_since_creation + ), + *self._get_conflicts_in_segment_ids_to_delete_overrides( + change_sets_since_creation + ), + ] + return conflicts + + def includes_change_to_environment_default(self) -> bool: + for fs_to_update in self.get_parsed_feature_states_to_update(): + if fs_to_update["feature_segment"] is None: + return True + + return False + + def includes_change_to_segment(self, segment_id: int) -> bool: + modified_segment_ids = { + *self.get_parsed_segment_ids_to_delete_overrides(), + *[ + fs["feature_segment"]["segment"] + for fs in filter( + lambda fs: fs["feature_segment"] is not None, + [ + *self.get_parsed_feature_states_to_create(), + *self.get_parsed_feature_states_to_update(), + ], + ) + ], + } + return segment_id in modified_segment_ids + + def _get_conflicts_in_feature_states_to_update( + self, change_sets_since_creation: list["VersionChangeSet"] + ) -> list[Conflict]: + _conflicts = [] + for fs_to_update in self.get_parsed_feature_states_to_update(): + feature_segment = fs_to_update["feature_segment"] + is_change_to_environment_default = feature_segment is None + + for change_set in change_sets_since_creation: + if is_change_to_environment_default: + if change_set.includes_change_to_environment_default(): + _conflicts.append( + Conflict( + original_cr_id=change_set.change_request_id, + published_at=change_set.published_at, + ) + ) + elif change_set.includes_change_to_segment(feature_segment["segment"]): + _conflicts.append( + Conflict( + original_cr_id=change_set.change_request_id, + segment_id=fs_to_update["feature_segment"]["segment"], + published_at=change_set.published_at, + ) + ) + + return _conflicts + + def _get_conflicts_in_feature_states_to_create( + self, change_sets_since_creation: list["VersionChangeSet"] + ) -> list[Conflict]: + _conflicts = [] + for fs_to_create in self.get_parsed_feature_states_to_create(): + for change_set in change_sets_since_creation: + # Note that feature states to create cannot be environment defaults so + # must always have a feature segment + if change_set.includes_change_to_segment( + fs_to_create["feature_segment"]["segment"] + ): + _conflicts.append( + Conflict( + original_cr_id=change_set.change_request_id, + segment_id=fs_to_create["feature_segment"]["segment"], + ) + ) + + return _conflicts + + def _get_conflicts_in_segment_ids_to_delete_overrides( + self, change_sets_since_create: list["VersionChangeSet"] + ) -> list[Conflict]: + _conflicts = [] + for segment_id in self.get_parsed_segment_ids_to_delete_overrides(): + for change_set in change_sets_since_create: + if change_set.includes_change_to_segment(segment_id): + _conflicts.append( + Conflict( + original_cr_id=change_set.change_request_id, + segment_id=segment_id, + ) + ) + + return _conflicts diff --git a/api/features/versioning/serializers.py b/api/features/versioning/serializers.py index 9774961b4274..ec21cdfae5ae 100644 --- a/api/features/versioning/serializers.py +++ b/api/features/versioning/serializers.py @@ -1,21 +1,25 @@ import typing +from django.core.exceptions import ObjectDoesNotExist from rest_framework import serializers from api_keys.user import APIKeyUser -from features.serializers import CreateSegmentOverrideFeatureStateSerializer +from features.serializers import ( + CustomCreateSegmentOverrideFeatureStateSerializer, +) from features.versioning.models import EnvironmentFeatureVersion from integrations.github.github import call_github_task +from segments.models import Segment from users.models import FFAdminUser from webhooks.webhooks import WebhookEventType -class EnvironmentFeatureVersionFeatureStateSerializer( - CreateSegmentOverrideFeatureStateSerializer +class CustomEnvironmentFeatureVersionFeatureStateSerializer( + CustomCreateSegmentOverrideFeatureStateSerializer ): - class Meta(CreateSegmentOverrideFeatureStateSerializer.Meta): + class Meta(CustomCreateSegmentOverrideFeatureStateSerializer.Meta): read_only_fields = ( - CreateSegmentOverrideFeatureStateSerializer.Meta.read_only_fields + CustomCreateSegmentOverrideFeatureStateSerializer.Meta.read_only_fields + ("feature",) ) @@ -91,7 +95,7 @@ def get_previous_version_uuid( class EnvironmentFeatureVersionCreateSerializer(EnvironmentFeatureVersionSerializer): - feature_states_to_create = EnvironmentFeatureVersionFeatureStateSerializer( + feature_states_to_create = CustomEnvironmentFeatureVersionFeatureStateSerializer( many=True, allow_null=True, required=False, @@ -101,7 +105,7 @@ class EnvironmentFeatureVersionCreateSerializer(EnvironmentFeatureVersionSeriali ), write_only=True, ) - feature_states_to_update = EnvironmentFeatureVersionFeatureStateSerializer( + feature_states_to_update = CustomEnvironmentFeatureVersionFeatureStateSerializer( many=True, allow_null=True, required=False, @@ -182,18 +186,23 @@ def _create_feature_state( if not self._is_segment_override(feature_state): raise serializers.ValidationError( { - "feature_states_to_create": "Cannot create FeatureState objects that are not segment overrides." + "message": "Cannot create FeatureState objects that are not segment overrides." } ) segment_id = feature_state["feature_segment"]["segment"] - if version.feature_states.filter( - feature_segment__segment_id=segment_id - ).exists(): + if ( + existing_segment_override := version.feature_states.filter( + feature_segment__segment_id=segment_id + ) + .select_related("feature_segment__segment") + .first() + ): raise serializers.ValidationError( { - "feature_states_to_create": "Segment override already exists for Segment %d" - % segment_id + "message": "An unresolvable conflict occurred: " + "segment override already exists for segment '%s'" + % existing_segment_override.feature_segment.segment.name } ) @@ -202,7 +211,7 @@ def _create_feature_state( "environment": version.environment, "environment_feature_version": version, } - fs_serializer = EnvironmentFeatureVersionFeatureStateSerializer( + fs_serializer = CustomEnvironmentFeatureVersionFeatureStateSerializer( data=feature_state, context=save_kwargs, ) @@ -213,9 +222,22 @@ def _update_feature_state( self, feature_state: dict[str, typing.Any], version: EnvironmentFeatureVersion ) -> None: if self._is_segment_override(feature_state): - instance = version.feature_states.get( - feature_segment__segment_id=feature_state["feature_segment"]["segment"] - ) + segment_id = feature_state["feature_segment"]["segment"] + try: + instance = version.feature_states.get( + feature_segment__segment_id=segment_id + ) + except ObjectDoesNotExist: + # Note that the segment will always exist because, if it didn't, + # it would have been picked up in the serializer validation. + segment = Segment.objects.get(id=segment_id) + raise serializers.ValidationError( + { + "message": "An unresolvable conflict occurred: " + "segment override does not exist for segment '%s'." + % segment.name + } + ) # Patch the id of the feature segment onto the feature state data so that # the serializer knows to update rather than try and create a new one. feature_state["feature_segment"]["id"] = instance.feature_segment_id @@ -238,7 +260,7 @@ def _update_feature_state( if updated_mvfsv_dict: updated_mvfsv_dict["id"] = existing_mvfsv.id - fs_serializer = EnvironmentFeatureVersionFeatureStateSerializer( + fs_serializer = CustomEnvironmentFeatureVersionFeatureStateSerializer( instance=instance, data=feature_state, context={ diff --git a/api/features/versioning/tasks.py b/api/features/versioning/tasks.py index c3f408d0cd3f..26a7ea8d9f40 100644 --- a/api/features/versioning/tasks.py +++ b/api/features/versioning/tasks.py @@ -1,6 +1,9 @@ import logging import typing +from django.conf import settings +from django.core.mail import send_mail +from django.template.loader import render_to_string from django.utils import timezone from task_processor.decorators import register_task_handler @@ -8,13 +11,18 @@ from audit.models import AuditLog from audit.related_object_type import RelatedObjectType from features.models import FeatureState -from features.versioning.models import EnvironmentFeatureVersion +from features.versioning.exceptions import FeatureVersioningError +from features.versioning.models import ( + EnvironmentFeatureVersion, + VersionChangeSet, +) from features.versioning.schemas import ( EnvironmentFeatureVersionWebhookDataSerializer, ) from features.versioning.versioning_service import ( get_environment_flags_queryset, ) +from users.models import FFAdminUser from webhooks.webhooks import WebhookEventType, call_environment_webhooks if typing.TYPE_CHECKING: @@ -153,3 +161,94 @@ def create_environment_feature_version_published_audit_log_task( author_id=environment_feature_version.published_by_id, master_api_key_id=environment_feature_version.published_by_api_key_id, ) + + +@register_task_handler() +def publish_version_change_set( + version_change_set_id: int, user_id: int, is_scheduled: bool = False +) -> None: + version_change_set = VersionChangeSet.objects.select_related( + "change_request", "change_request__user" + ).get(id=version_change_set_id) + user = FFAdminUser.objects.get(id=user_id) + + if is_scheduled and version_change_set.get_conflicts(): + _send_failed_due_to_conflict_alert_to_change_request_author(version_change_set) + return + + # Since the serializer is already able to handle this functionality, we re-use + # it in this task. + # TODO: in a separate PR, I'd like to refactor the version create endpoint to + # use change sets. At which point, we can revisit whether the serializer + # actually does the 'save'. + + # Note that, since the import path here eventually imports the + # djoser user serializer (which imports settings), we have to use + # a local import, since the tasks module gets loaded on app start, + # to avoid AppRegistryNotReady error. + from features.versioning.serializers import ( + EnvironmentFeatureVersionCreateSerializer, + ) + + serializer = EnvironmentFeatureVersionCreateSerializer( + data={ + "feature_states_to_create": ( + version_change_set.get_parsed_feature_states_to_create() + ), + "feature_states_to_update": ( + version_change_set.get_parsed_feature_states_to_update() + ), + "segment_ids_to_delete_overrides": ( + version_change_set.get_parsed_segment_ids_to_delete_overrides() + ), + } + ) + if not serializer.is_valid(): + logger.error( + "Unable to publish version change set. Serializer errors are: %s", + str(serializer.errors), + ) + raise FeatureVersioningError("Unable to publish version change set") + + version: EnvironmentFeatureVersion = serializer.save( + feature=version_change_set.feature, + environment=version_change_set.environment, + created_by=user, + ) + + now = timezone.now() + + # Note that we always set the live_from to `now` since we're publishing + # _now_. The VersionChangeSet might have been scheduled for the future + # 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) + + # if live_from was set on the version_change set, then leave it alone for + # auditing purposes. + if not version_change_set.live_from: + version_change_set.live_from = now + + version_change_set.published_at = now + version_change_set.published_by = user + version_change_set.environment_feature_version = version + version_change_set.save() + + +def _send_failed_due_to_conflict_alert_to_change_request_author( + version_change_set: VersionChangeSet, +) -> None: + context = { + "change_request": version_change_set.change_request, + "user": version_change_set.change_request.user, + "feature": version_change_set.feature, + } + send_mail( + subject=version_change_set.change_request.email_subject, + message=render_to_string( + "versioning/scheduled_change_failed_conflict_email.txt", context + ), + from_email=settings.DEFAULT_FROM_EMAIL, + recipient_list=[version_change_set.change_request.user.email], + ) diff --git a/api/features/versioning/templates/versioning/scheduled_change_failed_conflict_email.txt b/api/features/versioning/templates/versioning/scheduled_change_failed_conflict_email.txt new file mode 100644 index 000000000000..517c3e337bbc --- /dev/null +++ b/api/features/versioning/templates/versioning/scheduled_change_failed_conflict_email.txt @@ -0,0 +1,9 @@ +Hi {{ user.first_name | default:"there" }}, + +Your change to feature '{{ feature.name }}' as part of change request '{{ change_request.title }}' was not published +due to a conflict. + +Please review the change request and re-create the change. + +Thanks, +The Flagsmith team \ No newline at end of file diff --git a/api/features/versioning/views.py b/api/features/versioning/views.py index f4d1417e466f..184fb7266a7f 100644 --- a/api/features/versioning/views.py +++ b/api/features/versioning/views.py @@ -20,7 +20,9 @@ from environments.models import Environment from environments.permissions.constants import VIEW_ENVIRONMENT from features.models import Feature, FeatureState -from features.serializers import CreateSegmentOverrideFeatureStateSerializer +from features.serializers import ( + CustomCreateSegmentOverrideFeatureStateSerializer, +) from features.versioning.exceptions import FeatureVersionDeleteError from features.versioning.models import EnvironmentFeatureVersion from features.versioning.permissions import ( @@ -29,8 +31,8 @@ EnvironmentFeatureVersionRetrievePermissions, ) from features.versioning.serializers import ( + CustomEnvironmentFeatureVersionFeatureStateSerializer, EnvironmentFeatureVersionCreateSerializer, - EnvironmentFeatureVersionFeatureStateSerializer, EnvironmentFeatureVersionPublishSerializer, EnvironmentFeatureVersionQuerySerializer, EnvironmentFeatureVersionRetrieveSerializer, @@ -161,7 +163,7 @@ class EnvironmentFeatureVersionFeatureStatesViewSet( UpdateModelMixin, DestroyModelMixin, ): - serializer_class = EnvironmentFeatureVersionFeatureStateSerializer + serializer_class = CustomEnvironmentFeatureVersionFeatureStateSerializer permission_classes = [ IsAuthenticated, EnvironmentFeatureVersionFeatureStatePermissions, @@ -211,21 +213,25 @@ def get_serializer_context(self): context["environment_feature_version"] = self.environment_feature_version return context - def perform_create(self, serializer: CreateSegmentOverrideFeatureStateSerializer): + def perform_create( + self, serializer: CustomCreateSegmentOverrideFeatureStateSerializer + ) -> None: serializer.save( feature=self.feature, environment=self.environment, environment_feature_version=self.environment_feature_version, ) - def perform_update(self, serializer: CreateSegmentOverrideFeatureStateSerializer): + def perform_update( + self, serializer: CustomCreateSegmentOverrideFeatureStateSerializer + ) -> None: serializer.save( feature=self.feature, environment=self.environment, environment_feature_version=self.environment_feature_version, ) - def perform_destroy(self, instance): + def perform_destroy(self, instance) -> None: if instance.feature_segment is None and instance.identity is None: raise FeatureVersionDeleteError( "Cannot delete environment default feature state." diff --git a/api/features/views.py b/api/features/views.py index 889e4218c37f..ec0aad480f73 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -52,7 +52,7 @@ ) from .serializers import ( CreateFeatureSerializer, - CreateSegmentOverrideFeatureStateSerializer, + CustomCreateSegmentOverrideFeatureStateSerializer, FeatureEvaluationDataSerializer, FeatureGroupOwnerInputSerializer, FeatureInfluxDataSerializer, @@ -897,8 +897,8 @@ def organisation_has_got_feature(request, organisation): @swagger_auto_schema( method="POST", - request_body=CreateSegmentOverrideFeatureStateSerializer(), - responses={201: CreateSegmentOverrideFeatureStateSerializer()}, + request_body=CustomCreateSegmentOverrideFeatureStateSerializer(), + responses={201: CustomCreateSegmentOverrideFeatureStateSerializer()}, ) @api_view(["POST"]) @permission_classes([CreateSegmentOverridePermissions]) @@ -908,7 +908,7 @@ def create_segment_override( environment = get_object_or_404(Environment, api_key=environment_api_key) feature = get_object_or_404(Feature, project=environment.project, pk=feature_pk) - serializer = CreateSegmentOverrideFeatureStateSerializer( + serializer = CustomCreateSegmentOverrideFeatureStateSerializer( data=request.data, context={"environment": environment, "feature": feature} ) serializer.is_valid(raise_exception=True) diff --git a/api/features/workflows/core/models.py b/api/features/workflows/core/models.py index 83cfb5e4d33e..1e4c7d2ac9c1 100644 --- a/api/features/workflows/core/models.py +++ b/api/features/workflows/core/models.py @@ -111,6 +111,7 @@ def commit(self, committed_by: "FFAdminUser"): self._publish_feature_states() self._publish_environment_feature_versions(committed_by) + self._publish_change_sets(committed_by) self.committed_at = timezone.now() self.committed_by = committed_by @@ -174,6 +175,10 @@ def _publish_environment_feature_versions( EnvironmentFeatureVersion, instance=environment_feature_version ) + def _publish_change_sets(self, published_by: "FFAdminUser") -> None: + for change_set in self.change_sets.all(): + change_set.publish(user=published_by) + def get_create_log_message(self, history_instance) -> typing.Optional[str]: return CHANGE_REQUEST_CREATED_MESSAGE % self.title diff --git a/api/poetry.lock b/api/poetry.lock index edaf7055d593..1122e28f7734 100644 --- a/api/poetry.lock +++ b/api/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. [[package]] name = "annotated-types" @@ -1361,6 +1361,26 @@ url = "https://github.com/flagsmith/flagsmith-auth-controller" reference = "v0.1.2" resolved_reference = "d0f73840b4d5a078077c2bb108458356476d0ee5" +[[package]] +name = "flagsmith-common" +version = "0.1.0" +description = "A repository for including code that is required in multiple flagsmith repositories" +optional = false +python-versions = "^3.10" +files = [] +develop = false + +[package.dependencies] +django = "*" +djangorestframework = "*" +drf-writable-nested = "*" + +[package.source] +type = "git" +url = "https://github.com/Flagsmith/flagsmith-common" +reference = "v1.0.0" +resolved_reference = "f3809f6d592b2c6cfdfa88e0b345ce722ac47727" + [[package]] name = "flagsmith-flag-engine" version = "5.1.1" @@ -2159,16 +2179,6 @@ files = [ {file = "MarkupSafe-2.1.3-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:5bbe06f8eeafd38e5d0a4894ffec89378b6c6a625ff57e3028921f8ff59318ac"}, {file = "MarkupSafe-2.1.3-cp311-cp311-win32.whl", hash = "sha256:dd15ff04ffd7e05ffcb7fe79f1b98041b8ea30ae9234aed2a9168b5797c3effb"}, {file = "MarkupSafe-2.1.3-cp311-cp311-win_amd64.whl", hash = "sha256:134da1eca9ec0ae528110ccc9e48041e0828d79f24121a1a146161103c76e686"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-macosx_10_9_universal2.whl", hash = "sha256:f698de3fd0c4e6972b92290a45bd9b1536bffe8c6759c62471efaa8acb4c37bc"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:aa57bd9cf8ae831a362185ee444e15a93ecb2e344c8e52e4d721ea3ab6ef1823"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:ffcc3f7c66b5f5b7931a5aa68fc9cecc51e685ef90282f4a82f0f5e9b704ad11"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:47d4f1c5f80fc62fdd7777d0d40a2e9dda0a05883ab11374334f6c4de38adffd"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:1f67c7038d560d92149c060157d623c542173016c4babc0c1913cca0564b9939"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_aarch64.whl", hash = "sha256:9aad3c1755095ce347e26488214ef77e0485a3c34a50c5a5e2471dff60b9dd9c"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_i686.whl", hash = "sha256:14ff806850827afd6b07a5f32bd917fb7f45b046ba40c57abdb636674a8b559c"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8f9293864fe09b8149f0cc42ce56e3f0e54de883a9de90cd427f191c346eb2e1"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-win32.whl", hash = "sha256:715d3562f79d540f251b99ebd6d8baa547118974341db04f5ad06d5ea3eb8007"}, - {file = "MarkupSafe-2.1.3-cp312-cp312-win_amd64.whl", hash = "sha256:1b8dd8c3fd14349433c79fa8abeb573a55fc0fdd769133baac1f5e07abf54aeb"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:8e254ae696c88d98da6555f5ace2279cf7cd5b3f52be2b5cf97feafe883b58d2"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:cb0932dc158471523c9637e807d9bfb93e06a95cbf010f1a38b98623b929ef2b"}, {file = "MarkupSafe-2.1.3-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:9402b03f1a1b4dc4c19845e5c749e3ab82d5078d16a2a4c2cd2df62d57bb0707"}, @@ -3350,7 +3360,6 @@ files = [ {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:69b023b2b4daa7548bcfbd4aa3da05b3a74b772db9e23b982788168117739938"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:81e0b275a9ecc9c0c0c07b4b90ba548307583c125f54d5b6946cfee6360c733d"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:ba336e390cd8e4d1739f42dfe9bb83a3cc2e80f567d8805e11b46f4a943f5515"}, - {file = "PyYAML-6.0.1-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:326c013efe8048858a6d312ddd31d56e468118ad4cdeda36c719bf5bb6192290"}, {file = "PyYAML-6.0.1-cp310-cp310-win32.whl", hash = "sha256:bd4af7373a854424dabd882decdc5579653d7868b8fb26dc7d0e99f823aa5924"}, {file = "PyYAML-6.0.1-cp310-cp310-win_amd64.whl", hash = "sha256:fd1592b3fdf65fff2ad0004b5e363300ef59ced41c2e6b3a99d4089fa8c5435d"}, {file = "PyYAML-6.0.1-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:6965a7bc3cf88e5a1c3bd2e0b5c22f8d677dc88a455344035f03399034eb3007"}, @@ -3358,16 +3367,8 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:42f8152b8dbc4fe7d96729ec2b99c7097d656dc1213a3229ca5383f973a5ed6d"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:062582fca9fabdd2c8b54a3ef1c978d786e0f6b3a1510e0ac93ef59e0ddae2bc"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:d2b04aac4d386b172d5b9692e2d2da8de7bfb6c387fa4f801fbf6fb2e6ba4673"}, - {file = "PyYAML-6.0.1-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:e7d73685e87afe9f3b36c799222440d6cf362062f78be1013661b00c5c6f678b"}, {file = "PyYAML-6.0.1-cp311-cp311-win32.whl", hash = "sha256:1635fd110e8d85d55237ab316b5b011de701ea0f29d07611174a1b42f1444741"}, {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, - {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, - {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, - {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef"}, - {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, - {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, - {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, - {file = "PyYAML-6.0.1-cp312-cp312-win_amd64.whl", hash = "sha256:0d3304d8c0adc42be59c5f8a4d9e3d7379e6955ad754aa9d6ab7a398b59dd1df"}, {file = "PyYAML-6.0.1-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:50550eb667afee136e9a77d6dc71ae76a44df8b3e51e41b77f6de2932bfe0f47"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:1fe35611261b29bd1de0070f0b2f47cb6ff71fa6595c077e42bd0c419fa27b98"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:704219a11b772aea0d8ecd7058d0082713c3562b4e271b849ad7dc4a5c90c13c"}, @@ -3384,7 +3385,6 @@ files = [ {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a0cd17c15d3bb3fa06978b4e8958dcdc6e0174ccea823003a106c7d4d7899ac5"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:28c119d996beec18c05208a8bd78cbe4007878c6dd15091efb73a30e90539696"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:7e07cbde391ba96ab58e532ff4803f79c4129397514e1413a7dc761ccd755735"}, - {file = "PyYAML-6.0.1-cp38-cp38-musllinux_1_1_x86_64.whl", hash = "sha256:49a183be227561de579b4a36efbb21b3eab9651dd81b1858589f796549873dd6"}, {file = "PyYAML-6.0.1-cp38-cp38-win32.whl", hash = "sha256:184c5108a2aca3c5b3d3bf9395d50893a7ab82a38004c8f61c258d4428e80206"}, {file = "PyYAML-6.0.1-cp38-cp38-win_amd64.whl", hash = "sha256:1e2722cc9fbb45d9b87631ac70924c11d3a401b2d7f410cc0e3bbf249f2dca62"}, {file = "PyYAML-6.0.1-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:9eb6caa9a297fc2c2fb8862bc5370d0303ddba53ba97e71f08023b6cd73d16a8"}, @@ -3392,7 +3392,6 @@ files = [ {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:5773183b6446b2c99bb77e77595dd486303b4faab2b086e7b17bc6bef28865f6"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:b786eecbdf8499b9ca1d697215862083bd6d2a99965554781d0d8d1ad31e13a0"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:bc1bf2925a1ecd43da378f4db9e4f799775d6367bdb94671027b73b393a7c42c"}, - {file = "PyYAML-6.0.1-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:04ac92ad1925b2cff1db0cfebffb6ffc43457495c9b3c39d3fcae417d7125dc5"}, {file = "PyYAML-6.0.1-cp39-cp39-win32.whl", hash = "sha256:faca3bdcf85b2fc05d06ff3fbc1f83e1391b3e724afa3feba7d13eeab355484c"}, {file = "PyYAML-6.0.1-cp39-cp39-win_amd64.whl", hash = "sha256:510c9deebc5c0225e8c96813043e62b680ba2f9c50a08d3724c7f28a747d1486"}, {file = "PyYAML-6.0.1.tar.gz", hash = "sha256:bfdf460b1736c775f2ba9f6a92bca30bc2095067b8a9d77876d1fad6cc3b4a43"}, @@ -4071,13 +4070,14 @@ files = [] develop = false [package.dependencies] +flagsmith-common = {git = "https://github.com/Flagsmith/flagsmith-common", tag = "v1.0.0"} flagsmith-task-processor = {git = "https://github.com/Flagsmith/flagsmith-task-processor", tag = "v1.0.0"} [package.source] type = "git" url = "https://github.com/flagsmith/flagsmith-workflows" -reference = "v2.3.7" -resolved_reference = "1f963178c57a2889e8f8277d828e1e8875d49809" +reference = "v2.4.0" +resolved_reference = "d7549a451d551e8971c09782b6e2b7bb75a31fdc" [[package]] name = "wrapt" @@ -4196,4 +4196,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = ">=3.11, <3.13" -content-hash = "74a9b4c748ebc37ca08ce5d4b6f74ac57104c087e726d103470beb9d85304b34" +content-hash = "ce831ec68da148094062ddf59a2d851211144708a0ce0eace4c61bb81f9e94da" diff --git a/api/pyproject.toml b/api/pyproject.toml index b133a0648a94..c0426d72532b 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -170,6 +170,7 @@ hubspot-api-client = "^8.2.1" djangorestframework-dataclasses = "^1.3.1" pyotp = "^2.9.0" flagsmith-task-processor = { git = "https://github.com/Flagsmith/flagsmith-task-processor", tag = "v1.0.0" } +flagsmith-common = { git = "https://github.com/Flagsmith/flagsmith-common", tag = "v1.0.0" } [tool.poetry.group.auth-controller] optional = true @@ -193,7 +194,7 @@ flagsmith-ldap = { git = "https://github.com/flagsmith/flagsmith-ldap", tag = "v optional = true [tool.poetry.group.workflows.dependencies] -workflows-logic = { git = "https://github.com/flagsmith/flagsmith-workflows", tag = "v2.3.7" } +workflows-logic = { git = "https://github.com/flagsmith/flagsmith-workflows", tag = "v2.4.0" } [tool.poetry.group.dev.dependencies] django-test-migrations = "~1.2.0" diff --git a/api/tests/unit/features/versioning/test_unit_versioning_dataclasses.py b/api/tests/unit/features/versioning/test_unit_versioning_dataclasses.py new file mode 100644 index 000000000000..f7c89fd88efc --- /dev/null +++ b/api/tests/unit/features/versioning/test_unit_versioning_dataclasses.py @@ -0,0 +1,10 @@ +import pytest + +from features.versioning.dataclasses import Conflict + + +@pytest.mark.parametrize("segment_id, expected_result", ((None, True), (1, False))) +def test_conflict_is_environment_default( + segment_id: int | None, expected_result: bool +) -> None: + assert Conflict(segment_id=segment_id).is_environment_default is expected_result 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 ee26cc905e4f..771b2480667d 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_models.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_models.py @@ -1,23 +1,24 @@ -import typing +import json from datetime import timedelta import pytest +from core.constants import STRING from django.utils import timezone from freezegun import freeze_time +from pytest_mock import MockerFixture from environments.models import Environment from environments.tasks import rebuild_environment_document -from features.models import FeatureSegment, FeatureState +from features.models import Feature, FeatureSegment, FeatureState from features.versioning.exceptions import FeatureVersioningError -from features.versioning.models import EnvironmentFeatureVersion +from features.versioning.models import ( + EnvironmentFeatureVersion, + VersionChangeSet, +) +from features.workflows.core.models import ChangeRequest +from projects.models import Project from segments.models import Segment - -if typing.TYPE_CHECKING: - from pytest_mock import MockerFixture - - from features.models import Feature - from projects.models import Project - from users.models import FFAdminUser +from users.models import FFAdminUser now = timezone.now() @@ -277,3 +278,408 @@ def test_get_latest_versions_does_not_return_versions_scheduled_for_the_future( # Then assert latest_versions.count() == 1 assert latest_versions.first() == version_0 + + +def test_version_change_set_adds_environment_on_create_with_change_request( + environment_v2_versioning: Environment, + feature: "Feature", + change_request: ChangeRequest, +) -> None: + # Given + version_change_set = VersionChangeSet( + feature=feature, change_request=change_request + ) + + # When + version_change_set.save() + + # Then + assert version_change_set.environment == change_request.environment + + +def test_version_change_set_adds_environment_on_create_with_environment_feature_version( + environment_v2_versioning: Environment, feature: "Feature" +) -> None: + # Given + version = EnvironmentFeatureVersion.objects.create( + environment=environment_v2_versioning, feature=feature + ) + version_change_set = VersionChangeSet( + feature=feature, environment_feature_version=version + ) + + # When + version_change_set.save() + + # Then + assert version_change_set.environment == version.environment + + +def test_version_change_set_create_fails_if_no_related_object( + feature: "Feature", +) -> None: + # Given + version_change_set = VersionChangeSet(feature=feature) + + # When + with pytest.raises(RuntimeError) as e: + version_change_set.save() + + # Then + assert ( + str(e.value) + == "Version change set should belong to either a change request, or a version." + ) + + +def test_version_change_set_get_conflicts( + project: Project, + segment: Segment, + another_segment: Segment, + feature: "Feature", + segment_featurestate: FeatureState, + another_segment_featurestate: FeatureState, + environment_v2_versioning: Environment, + admin_user: "FFAdminUser", +) -> None: + # Given + # We create 4 change sets... + # ... one to modify the environment default feature state + change_request_1 = ChangeRequest.objects.create( + environment=environment_v2_versioning, title="Test CR 1" + ) + VersionChangeSet.objects.create( + change_request=change_request_1, + feature=feature, + feature_states_to_update=json.dumps( + [ + { + "feature_segment": None, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "value from version change set 1", + }, + } + ] + ), + feature_states_to_create="[]", + segment_ids_to_delete_overrides="[]", + ) + + # ... one to create a new segment override for a new segment + new_segment = Segment.objects.create(name="new segment", project=project) + change_request_2 = ChangeRequest.objects.create( + environment=environment_v2_versioning, title="Test CR 2" + ) + VersionChangeSet.objects.create( + change_request=change_request_2, + feature=feature, + feature_states_to_update="[]", + feature_states_to_create=json.dumps( + [ + { + "feature_segment": {"segment": new_segment.id}, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "segment override value from version change set 2", + }, + } + ] + ), + segment_ids_to_delete_overrides="[]", + ) + + # ... and one to remove the segment override for the first segment + change_request_3 = ChangeRequest.objects.create( + environment=environment_v2_versioning, title="Test CR 3" + ) + VersionChangeSet.objects.create( + change_request=change_request_3, + feature=feature, + feature_states_to_update="[]", + feature_states_to_create="[]", + segment_ids_to_delete_overrides=json.dumps([segment.id]), + ) + + # ... and one to update the segment override for the second segment + change_request_4 = ChangeRequest.objects.create( + environment=environment_v2_versioning, title="Test CR 4" + ) + VersionChangeSet.objects.create( + change_request=change_request_4, + feature=feature, + feature_states_to_update=json.dumps( + [ + { + "feature_segment": {"segment": another_segment.id}, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "segment override value from version change set 4", + }, + } + ] + ), + feature_states_to_create="[]", + segment_ids_to_delete_overrides="[]", + ) + + # Finally, we create another change set to modify all the same things as each individual + # change set that we already defined above. + primary_change_request = ChangeRequest.objects.create( + environment=environment_v2_versioning, title="Test CR" + ) + primary_version_change_set = VersionChangeSet.objects.create( + change_request=primary_change_request, + feature=feature, + feature_states_to_update=json.dumps( + [ + { + "feature_segment": None, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "primary value", + }, + }, + { + "feature_segment": {"segment": another_segment.id}, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "segment override value from primary change set", + }, + }, + ] + ), + feature_states_to_create=json.dumps( + [ + { + "feature_segment": {"segment": new_segment.id}, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "segment override value from primary change set", + }, + } + ] + ), + segment_ids_to_delete_overrides=json.dumps([segment.id]), + ) + + # And we publish the first 4 change sets (via the change requests) + for cr in [change_request_1, change_request_2, change_request_3, change_request_4]: + cr.commit(admin_user) + + # When + conflicts = primary_version_change_set.get_conflicts() + + # Then + assert len(conflicts) == 4 + + # build a dictionary keyed off the original_cr_id to make the + # assertions below easier. + conflict_dict = {c.original_cr_id: c for c in conflicts} + + assert conflict_dict[change_request_1.id].segment_id is None + assert conflict_dict[change_request_1.id].is_environment_default + + assert conflict_dict[change_request_2.id].segment_id is new_segment.id + assert not conflict_dict[change_request_2.id].is_environment_default + + assert conflict_dict[change_request_3.id].segment_id is segment.id + assert not conflict_dict[change_request_3.id].is_environment_default + + assert conflict_dict[change_request_4.id].segment_id is another_segment.id + assert not conflict_dict[change_request_4.id].is_environment_default + + +def test_version_change_set_publish_not_scheduled( + environment_v2_versioning: Environment, + change_request: ChangeRequest, + feature: Feature, + mocker: MockerFixture, + admin_user: FFAdminUser, +) -> None: + # Given + mock_publish_version_change_set = mocker.patch( + "features.versioning.tasks.publish_version_change_set" + ) + + version_change_set = VersionChangeSet.objects.create( + change_request=change_request, + feature=feature, + ) + + # When + version_change_set.publish(admin_user) + + # Then + mock_publish_version_change_set.assert_called_once_with( + version_change_set_id=version_change_set.id, user_id=admin_user.id + ) + + +def test_version_change_set_publish_scheduled( + environment_v2_versioning: Environment, + change_request: ChangeRequest, + feature: Feature, + mocker: MockerFixture, + admin_user: FFAdminUser, +) -> None: + # Given + mock_publish_version_change_set = mocker.patch( + "features.versioning.tasks.publish_version_change_set" + ) + + live_from = timezone.now() + timedelta(days=1) + version_change_set = VersionChangeSet.objects.create( + change_request=change_request, + feature=feature, + live_from=live_from, + ) + + # When + version_change_set.publish(admin_user) + + # Then + mock_publish_version_change_set.delay.assert_called_once_with( + kwargs={ + "version_change_set_id": version_change_set.id, + "user_id": admin_user.id, + "is_scheduled": True, + }, + delay_until=live_from, + ) + + +def test_version_change_set_get_conflicts_returns_empty_list_if_published( + feature: Feature, + admin_user: FFAdminUser, + segment: Segment, + segment_featurestate: FeatureState, + environment_v2_versioning: Environment, +) -> None: + # Given + # We create 2 change requests (with change sets) that operate on the + # same segment override and publish them both + change_sets = [] + for i in range(1, 3): + change_set = VersionChangeSet.objects.create( + change_request=ChangeRequest.objects.create( + title=f"CR{i}", + environment=environment_v2_versioning, + ), + feature=feature, + feature_states_to_update=json.dumps( + [ + { + "feature_segment": {"segment": segment.id}, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": f"override value {i}", + }, + } + ] + ), + ) + change_set.change_request.commit(admin_user) + change_set.refresh_from_db() + change_sets.append(change_set) + + # Then + assert all(cs.get_conflicts() == [] for cs in change_sets) + + +def test_version_change_set_get_conflicts_returns_empty_list_if_no_conflicts( + change_request: ChangeRequest, + feature: Feature, + segment: Segment, + admin_user: FFAdminUser, + environment_v2_versioning: Environment, +) -> None: + # Given + # We create 2 change requests (with change sets) that operate on different + # elements of a given feature (one creates a segment override, one + # updates the environment default) + segment_override_change_set = VersionChangeSet.objects.create( + change_request=ChangeRequest.objects.create( + title="Segment Override CR", + environment=environment_v2_versioning, + ), + feature=feature, + feature_states_to_create=json.dumps( + [ + { + "feature_segment": {"segment": segment.id}, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "override value", + }, + } + ] + ), + ) + + environment_default_change_set = VersionChangeSet.objects.create( + change_request=ChangeRequest.objects.create( + title="Segment Override CR", + environment=environment_v2_versioning, + ), + feature=feature, + feature_states_to_update=json.dumps( + [ + { + "feature_segment": None, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "updated value", + }, + } + ] + ), + ) + + # And we publish the segment override change set + segment_override_change_set.change_request.commit(admin_user) + segment_override_change_set.refresh_from_db() + + # When + conflicts = environment_default_change_set.get_conflicts() + + # Then + assert conflicts == [] + + +def test_version_change_set_get_conflicts_returns_empty_list_if_no_change_sets_since_creation( + change_request: ChangeRequest, + feature: Feature, +) -> None: + # Given + change_set = VersionChangeSet.objects.create( + change_request=change_request, + feature=feature, + feature_states_to_update=json.dumps( + [ + { + "feature_segment": None, + "enabled": True, + "feature_state_value": { + "type": STRING, + "string_value": "updated value", + }, + } + ] + ), + ) + + # When + conflicts = change_set.get_conflicts() + + # Then + assert conflicts == [] 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 d15867ae3ad1..6568a78b1cb2 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_tasks.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_tasks.py @@ -1,21 +1,33 @@ import json from datetime import timedelta +from unittest import mock import freezegun +import pytest import responses +from core.constants import STRING +from django.core.mail import EmailMessage +from django.template.loader import render_to_string from django.utils import timezone -from pytest_mock import MockerFixture +from freezegun import freeze_time +from rest_framework.exceptions import ValidationError from environments.identities.models import Identity from environments.models import Environment, Webhook from features.models import Feature, FeatureSegment, FeatureState -from features.versioning.models import EnvironmentFeatureVersion +from features.versioning.exceptions import FeatureVersioningError +from features.versioning.models import ( + EnvironmentFeatureVersion, + VersionChangeSet, +) from features.versioning.tasks import ( disable_v2_versioning, enable_v2_versioning, + publish_version_change_set, trigger_update_version_webhooks, ) from features.versioning.versioning_service import ( + get_environment_flags_dict, get_environment_flags_queryset, ) from features.workflows.core.models import ChangeRequest @@ -144,7 +156,7 @@ def test_disable_v2_versioning( @responses.activate def test_trigger_update_version_webhooks( - environment_v2_versioning: Environment, feature: Feature, mocker: MockerFixture + environment_v2_versioning: Environment, feature: Feature ) -> None: # Given version = EnvironmentFeatureVersion.objects.get( @@ -246,3 +258,209 @@ def test_enable_v2_versioning_for_scheduled_changes( environment_flags_queryset_two_hours_later.first() == scheduled_feature_state ) + + +def test_publish_version_change_set_sends_email_to_change_request_owner_if_conflicts_when_scheduled( + feature: Feature, + environment_v2_versioning: Environment, + staff_user: FFAdminUser, + mailoutbox: list[EmailMessage], +) -> None: + # Given + live_from = timezone.now() + timedelta(hours=1) + + # First, we need to create the change request, and change set + # that we want to publish in this test (but should fail because + # of a conflict with another change set that we will create + # afterwards and publish immediately). + change_request_1 = ChangeRequest.objects.create( + title="CR 1", + environment=environment_v2_versioning, + user=staff_user, + ) + change_set_to_publish = VersionChangeSet.objects.create( + feature=feature, + change_request=change_request_1, + live_from=live_from, + feature_states_to_update=json.dumps( + [ + { + "feature": feature.id, + "enabled": True, + "feature_segment": None, + "feature_state_value": {"type": "unicode", "string_value": "foo"}, + } + ] + ), + ) + with mock.patch( + "features.versioning.tasks.publish_version_change_set" + ) as mock_publish: + change_request_1.commit(staff_user) + task_kwargs = { + "version_change_set_id": change_set_to_publish.id, + "user_id": staff_user.id, + "is_scheduled": True, + } + mock_publish.delay.assert_called_once_with( + kwargs=task_kwargs, + delay_until=live_from, + ) + + # Now, we'll create another change request and change set that will + # conflict with the first one and publish it immediately. + conflict_change_request = ChangeRequest.objects.create( + title="Conflict CR", + environment=environment_v2_versioning, + user=staff_user, + ) + conflict_feature_value = "bar" + conflict_feature_enabled = False + VersionChangeSet.objects.create( + feature=feature, + change_request=conflict_change_request, + feature_states_to_update=json.dumps( + [ + { + "feature": feature.id, + "enabled": conflict_feature_enabled, + "feature_segment": None, + "feature_state_value": { + "type": "unicode", + "string_value": conflict_feature_value, + }, + } + ] + ), + ) + conflict_change_request.commit(staff_user) + + # When + # We simulate the task being called by the task processor at the correct time, + # as per the mock call that we asserted above. + with freezegun.freeze_time(live_from): + publish_version_change_set(**task_kwargs) + + # Then + # an alert was sent to the change request owner + assert len(mailoutbox) == 1 + assert mailoutbox[0].subject == change_request_1.email_subject + assert mailoutbox[0].to == [staff_user.email] + assert mailoutbox[0].body == render_to_string( + "versioning/scheduled_change_failed_conflict_email.txt", + context={ + "change_request": change_request_1, + "user": staff_user, + "feature": feature, + }, + ) + + # and the change is not reflected in the flags + latest_flags = get_environment_flags_dict(environment=environment_v2_versioning) + assert latest_flags[(feature.id, None, None)].enabled is conflict_feature_enabled + assert ( + latest_flags[(feature.id, None, None)].get_feature_state_value() + == conflict_feature_value + ) + + +def test_publish_version_change_set_raises_error_when_segment_override_does_not_exist( + change_request: ChangeRequest, + environment_v2_versioning: Environment, + feature: Feature, + segment: Segment, + admin_user: FFAdminUser, +) -> None: + # Given + # We create a change set that attempts to update a segment override that + # doesn't currently exist. + change_set = VersionChangeSet.objects.create( + change_request=change_request, + feature=feature, + feature_states_to_update=json.dumps( + [ + { + "feature_segment": {"segment": segment.id}, + "enabled": True, + "feature_state_value": {"type": STRING, "string_value": "override"}, + } + ] + ), + ) + + # When + with pytest.raises(ValidationError) as e: + publish_version_change_set( + version_change_set_id=change_set.id, user_id=admin_user.id + ) + + # Then + assert e.value.detail["message"] == ( + f"An unresolvable conflict occurred: segment override does not exist for segment '{segment.name}'." + ) + + +def test_publish_version_change_set_raises_error_when_serializer_not_valid( + change_request: ChangeRequest, + environment_v2_versioning: Environment, + feature: Feature, + segment: Segment, + admin_user: FFAdminUser, + caplog: pytest.LogCaptureFixture, +) -> None: + # Given + # We create a change set for which the JSON data is invalid. + # Note that this should never happen since the data is validated on create. + change_set = VersionChangeSet.objects.create( + change_request=change_request, feature=feature, feature_states_to_update="[{}]" + ) + + # When + with pytest.raises(FeatureVersioningError) as e: + publish_version_change_set( + version_change_set_id=change_set.id, user_id=admin_user.id + ) + + # Then + assert str(e.value) == "Unable to publish version change set" + + +def test_publish_version_change_set_uses_current_time_for_version_live_from( + change_request: ChangeRequest, + feature: Feature, + environment_v2_versioning: Environment, + admin_user: FFAdminUser, +) -> None: + # Given + now = timezone.now() + five_minutes_ago = now - timezone.timedelta(minutes=5) + + # a version change_set that sets a live_from a short time in the past + VersionChangeSet.objects.create( + change_request=change_request, + feature=feature, + feature_states_to_update=json.dumps( + [ + { + "feature_segment": None, + "enabled": True, + "feature_state_value": {"type": STRING, "string_value": "foo"}, + } + ] + ), + live_from=five_minutes_ago, + ) + + # When + with freeze_time(now): + change_request.commit(admin_user) + + # Then + assert ( + EnvironmentFeatureVersion.objects.get_latest_versions_as_queryset( + environment_v2_versioning.id + ) + .get(feature=feature) + .live_from + == now + ) 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 09e42cfc2552..b88b12b26589 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_views.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_views.py @@ -984,7 +984,7 @@ def test_create_environment_default_when_creating_new_version_fails( assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json() == { - "feature_states_to_create": "Cannot create FeatureState objects that are not segment overrides." + "message": "Cannot create FeatureState objects that are not segment overrides." } @@ -1022,8 +1022,8 @@ def test_create_segment_override_for_existing_override_when_creating_new_version # Then assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json() == { - "feature_states_to_create": "Segment override already exists for Segment %d" - % segment.id + "message": "An unresolvable conflict occurred: segment override already exists for segment '%s'" + % segment.name }