Skip to content

Commit d9bf73d

Browse files
committed
Reject null environment with a validation error
Address review feedback: raise ValidationError instead of passing null through, since the nullable environment column is a historical data layer artefact and not intended API behaviour.
1 parent 82ad887 commit d9bf73d

File tree

2 files changed

+7
-4
lines changed

2 files changed

+7
-4
lines changed

api/features/serializers.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,9 @@ def validate_feature(self, feature): # type: ignore[no-untyped-def]
598598

599599
def validate_environment(self, environment): # type: ignore[no-untyped-def]
600600
if environment is None:
601-
return environment
601+
raise serializers.ValidationError(
602+
"This field may not be null."
603+
)
602604
if self.instance and self.instance.environment_id != environment.id: # type: ignore[union-attr]
603605
raise serializers.ValidationError(
604606
"Cannot change the environment of a feature state"

api/tests/unit/features/test_unit_features_serializers.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from features.serializers import FeatureStateSerializerBasic
1515

1616

17-
def test_feature_state_serializer_basic__null_environment_with_context__is_valid( # type: ignore[no-untyped-def]
17+
def test_feature_state_serializer_basic__null_environment__returns_validation_error( # type: ignore[no-untyped-def]
1818
feature, environment
1919
):
2020
# Given
@@ -33,8 +33,9 @@ def test_feature_state_serializer_basic__null_environment_with_context__is_valid
3333
# When
3434
is_valid = serializer.is_valid()
3535

36-
# Then - should not raise AttributeError on environment.id
37-
assert is_valid
36+
# Then - should reject null environment, not raise AttributeError
37+
assert not is_valid
38+
assert "environment" in serializer.errors
3839

3940

4041
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)