Skip to content

Commit

Permalink
fix(model/featurestate): make environment not null (#2708)
Browse files Browse the repository at this point in the history
  • Loading branch information
gagantrivedi committed Sep 5, 2023
1 parent e71efbb commit 55a9ef7
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Generated by Django 3.2.20 on 2023-08-30 13:14

from django.db import migrations, models
import django.db.models.deletion


def delete_feature_states_without_environment(apps, schema_editor):
FeatureState = apps.get_model("features", "FeatureState")
FeatureState.objects.filter(environment__isnull=True).delete()


class Migration(migrations.Migration):
atomic = False
dependencies = [
(
"environments",
"0032_rename_use_mv_v2_evaluation_to_use_in_percentage_split_evaluation",
),
("features", "0059_fix_feature_type"),
]

operations = [
migrations.RunPython(
delete_feature_states_without_environment,
reverse_code=migrations.RunPython.noop,
),
migrations.AlterField(
model_name="featurestate",
name="environment",
field=models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="feature_states",
to="environments.environment",
),
),
]
1 change: 0 additions & 1 deletion api/features/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,6 @@ class FeatureState(
environment = models.ForeignKey(
"environments.Environment",
related_name="feature_states",
null=True,
on_delete=models.CASCADE,
)
identity = models.ForeignKey(
Expand Down
60 changes: 60 additions & 0 deletions api/tests/unit/features/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import pytest
from django.conf import settings
from django.db.utils import IntegrityError
from django.utils import timezone

from features.feature_types import MULTIVARIATE, STANDARD
Expand Down Expand Up @@ -241,3 +242,62 @@ def test_fix_feature_type_migration(migrator):
)
assert NewFeature.objects.get(id=standard_feature.id).type == STANDARD
assert NewFeature.objects.get(id=mv_feature.id).type == MULTIVARIATE


@pytest.mark.skipif(
settings.SKIP_MIGRATION_TESTS is True,
reason="Skip migration tests to speed up tests where necessary",
)
def test_migrate_set_featurestate_environment_not_null(migrator):
# Given
old_state = migrator.apply_initial_migration(("features", "0059_fix_feature_type"))

Organisation = old_state.apps.get_model("organisations", "Organisation")
Project = old_state.apps.get_model("projects", "Project")
Environment = old_state.apps.get_model("environments", "Environment")
Feature = old_state.apps.get_model("features", "Feature")
FeatureState = old_state.apps.get_model("features", "FeatureState")

organisation = Organisation.objects.create(name="test org")
project = Project.objects.create(
name="test project", organisation_id=organisation.id
)
environment = Environment.objects.create(
name="test environment", project_id=project.id
)
feature = Feature.objects.create(name="test_feature", project_id=project.id)

# mimick the creation of the feature states that would have happened when save is called on the model (but doesn't
# happen because we're using the migrator models)
FeatureState.objects.create(feature=feature, environment=environment, enabled=True)

# Let's create a feature state with a null environment
fs_without_environment = FeatureState.objects.create(
feature_id=feature.id,
environment_id=None,
enabled=True,
)

# When
new_state = migrator.apply_tested_migration(
("features", "0060_set_featurestate_environment_not_null")
)

# Then
NewFeatureState = new_state.apps.get_model("features", "FeatureState")

# The feature state should have been deleted
assert not NewFeatureState.objects.filter(id=fs_without_environment.id).exists()

# feature state with environment still exists
assert NewFeatureState.objects.filter(
feature_id=feature.id, environment_id=environment.id
).exists()

# and we cant create a feature state without an environment
with pytest.raises(IntegrityError):
NewFeatureState.objects.create(
feature_id=feature.id,
environment_id=None,
enabled=True,
)

3 comments on commit 55a9ef7

@vercel
Copy link

@vercel vercel bot commented on 55a9ef7 Sep 5, 2023

Choose a reason for hiding this comment

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

@vercel
Copy link

@vercel vercel bot commented on 55a9ef7 Sep 5, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

docs – ./docs

docs-flagsmith.vercel.app
docs-git-main-flagsmith.vercel.app
docs.flagsmith.com
docs.bullet-train.io

@vercel
Copy link

@vercel vercel bot commented on 55a9ef7 Sep 5, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.