Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow ignore conflicts on scheduled change #4590

Merged
merged 3 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion api/features/versioning/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ def publish_version_change_set(
).get(id=version_change_set_id)
user = FFAdminUser.objects.get(id=user_id)

if is_scheduled and version_change_set.get_conflicts():
ignore_conflicts = version_change_set.change_request.ignore_conflicts
if not ignore_conflicts and is_scheduled and version_change_set.get_conflicts():
_send_failed_due_to_conflict_alert_to_change_request_author(version_change_set)
return

Expand Down Expand Up @@ -208,6 +209,10 @@ def publish_version_change_set(
"Unable to publish version change set. Serializer errors are: %s",
str(serializer.errors),
)
if is_scheduled:
_send_failed_due_to_conflict_alert_to_change_request_author(
version_change_set
)
raise FeatureVersioningError("Unable to publish version change set")

version: EnvironmentFeatureVersion = serializer.save(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 4.2.15 on 2024-09-05 08:11

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("workflows_core", "0009_prevent_cascade_delete_from_user_delete"),
]

operations = [
migrations.AddField(
model_name="changerequest",
name="ignore_conflicts",
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name="historicalchangerequest",
name="ignore_conflicts",
field=models.BooleanField(default=False),
),
]
2 changes: 2 additions & 0 deletions api/features/workflows/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class ChangeRequest(
null=True,
)

ignore_conflicts = models.BooleanField(default=False)

def approve(self, user: "FFAdminUser"):
if user.id == self.user_id:
raise CannotApproveOwnChangeRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.template.loader import render_to_string
from django.utils import timezone
from freezegun import freeze_time
from freezegun.api import FrozenDateTimeFactory
from rest_framework.exceptions import ValidationError

from environments.identities.models import Identity
Expand Down Expand Up @@ -464,3 +465,53 @@ def test_publish_version_change_set_uses_current_time_for_version_live_from(
.live_from
== now
)


def test_scheduled_versioning_change_set_with_ignore_conflicts_sends_email_if_validation_fails(
feature: Feature,
environment_v2_versioning: Environment,
admin_user: FFAdminUser,
freezer: FrozenDateTimeFactory,
mailoutbox: list[EmailMessage],
) -> None:
# Given
now = timezone.now()
five_minutes_from_now = now + timezone.timedelta(minutes=5)

change_request = ChangeRequest.objects.create(
title="Test CR",
environment=environment_v2_versioning,
user=admin_user,
ignore_conflicts=True,
)
change_set = VersionChangeSet.objects.create(
change_request=change_request,
feature=feature,
feature_states_to_update=json.dumps(
[{"foo": "bar"}]
), # bad data to force serializer validation failure
live_from=five_minutes_from_now,
)
change_request.commit(admin_user)

# When
freezer.move_to(five_minutes_from_now)
with pytest.raises(FeatureVersioningError):
publish_version_change_set(
version_change_set_id=change_set.id,
user_id=admin_user.id,
is_scheduled=True,
)

# Then
assert len(mailoutbox) == 1
assert mailoutbox[0].subject == change_request.email_subject
assert mailoutbox[0].to == [admin_user.email]
assert mailoutbox[0].body == render_to_string(
"versioning/scheduled_change_failed_conflict_email.txt",
context={
"user": admin_user,
"feature": feature,
"change_request": change_request,
},
)
124 changes: 124 additions & 0 deletions api/tests/unit/features/workflows/core/test_unit_workflows_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
import freezegun
import pytest
from django.contrib.sites.models import Site
from django.db.models import Q
from django.utils import timezone
from flag_engine.segments.constants import PERCENTAGE_SPLIT
from freezegun.api import FrozenDateTimeFactory
from pytest_mock import MockerFixture

from audit.constants import (
Expand All @@ -22,6 +25,7 @@
EnvironmentFeatureVersion,
VersionChangeSet,
)
from features.versioning.tasks import publish_version_change_set
from features.versioning.versioning_service import get_environment_flags_list
from features.workflows.core.exceptions import (
CannotApproveOwnChangeRequest,
Expand All @@ -33,6 +37,8 @@
ChangeRequestApproval,
ChangeRequestGroupAssignment,
)
from projects.models import Project
from segments.models import Condition, Segment, SegmentRule
from users.models import FFAdminUser

now = timezone.now()
Expand Down Expand Up @@ -773,3 +779,121 @@ def test_change_request_live_from_for_change_request_with_change_set(

# Then
assert change_request.live_from == now


def test_ignore_conflicts_for_multiple_scheduled_change_requests(
feature: Feature,
environment_v2_versioning: Environment,
admin_user: FFAdminUser,
project: Project,
freezer: FrozenDateTimeFactory,
mocker: MockerFixture,
) -> None:
"""
This test is for the specific use case where we want to schedule a slow
roll-out for a feature.
"""
# We are going to simulate the task processor ourselves, so let's mock it so we can assert
# the required calls later.
mock_publish_version_change_set = mocker.patch(
"features.versioning.tasks.publish_version_change_set"
)

# First, let's create the 2 percentage split segments that we want to use for the roll-out
def _create_segment(percentage_value: int) -> Segment:
segment = Segment.objects.create(
name=f"percentage_split_segment_{percentage_value}", project=project
)
parent_rule = SegmentRule.objects.create(
segment=segment, type=SegmentRule.ALL_RULE
)
child_rule = SegmentRule.objects.create(
rule=parent_rule, type=SegmentRule.ANY_RULE
)
Condition.objects.create(
rule=child_rule, property=PERCENTAGE_SPLIT, value=str(percentage_value)
)
return segment

ten_percent_segment = _create_segment(10)
twenty_percent_segment = _create_segment(20)

now = timezone.now()
ten_minutes_from_now = now + timedelta(minutes=10)
twenty_minutes_from_now = now + timedelta(minutes=20)

# Now, let's create our change requests to create the 2 overrides in the future
change_requests = []
for segment_to_add, live_from, segments_to_delete in [
(ten_percent_segment, ten_minutes_from_now, []),
(twenty_percent_segment, twenty_minutes_from_now, [ten_percent_segment]),
]:
change_request = ChangeRequest.objects.create(
title="Scheduled CR1",
environment=environment_v2_versioning,
user=admin_user,
ignore_conflicts=True,
)
version_change_set = VersionChangeSet.objects.create(
change_request=change_request,
feature=feature,
feature_states_to_create=json.dumps(
[
{
"feature_segment": {
"segment": segment_to_add.id,
},
"enabled": True,
"feature_state_value": {"type": "unicode", "string_value": ""},
}
]
),
segment_ids_to_delete_overrides=json.dumps(
[s.id for s in segments_to_delete]
),
live_from=live_from,
)
change_requests.append(change_request)
change_request.commit(committed_by=admin_user)
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,
)
mock_publish_version_change_set.reset_mock()

mock_publish_version_change_set.stop()

# Now, let's move time forward and publish the first change request (note: this is
# simulating the task processor picking up the task that we mock asserted earlier)
freezer.move_to(ten_minutes_from_now)
publish_version_change_set(
version_change_set_id=change_requests[0].change_sets.first().id,
user_id=admin_user.id,
is_scheduled=True,
)

after_cr_1_flags = get_environment_flags_list(
environment=environment_v2_versioning,
additional_filters=Q(feature_segment__isnull=False),
)
assert len(after_cr_1_flags) == 1
assert after_cr_1_flags[0].feature_segment.segment == ten_percent_segment

# Now, let's move time forward again and publish the second change request
freezer.move_to(twenty_minutes_from_now)
publish_version_change_set(
version_change_set_id=change_requests[1].change_sets.first().id,
user_id=admin_user.id,
is_scheduled=True,
)

after_cr_1_flags = get_environment_flags_list(
environment=environment_v2_versioning,
additional_filters=Q(feature_segment__isnull=False),
)
assert len(after_cr_1_flags) == 1
assert after_cr_1_flags[0].feature_segment.segment == twenty_percent_segment
Loading