Skip to content

Commit 6fb8e9b

Browse files
committed
refactor: add set_value method to clear stale fields
The _update_feature_state_value function was setting new typed values without clearing old value fields when changing types. This left stale data in the database. Added set_value() method to AbstractBaseFeatureValueModel that centralizes validation, type conversion, and ensures unused fields are cleared.
1 parent 5627624 commit 6fb8e9b

File tree

3 files changed

+37
-41
lines changed

3 files changed

+37
-41
lines changed

api/features/feature_states/models.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,26 @@ def value(self) -> typing.Union[str, int, bool]:
3838
self.type, # type: ignore[arg-type]
3939
self.string_value,
4040
)
41+
42+
def set_value(self, value: str, type_: str) -> None:
43+
self.string_value = None
44+
self.integer_value = None
45+
self.boolean_value = None
46+
47+
match type_:
48+
case "string":
49+
self.string_value = value
50+
self.type = STRING
51+
case "integer":
52+
try:
53+
self.integer_value = int(value)
54+
except ValueError:
55+
raise ValueError(f"'{value}' is not a valid integer")
56+
self.type = INTEGER
57+
case "boolean":
58+
if value.lower() not in ("true", "false"):
59+
raise ValueError(
60+
f"'{value}' is not a valid boolean (use 'true' or 'false')"
61+
)
62+
self.boolean_value = value.lower() == "true"
63+
self.type = BOOLEAN

api/features/versioning/versioning_service.py

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
from django.db.models import Prefetch, Q, QuerySet
55
from django.utils import timezone
66

7-
from core.constants import BOOLEAN, INTEGER, STRING
87
from environments.models import Environment
98
from features.models import Feature, FeatureState, FeatureStateValue
109
from features.versioning.dataclasses import (
@@ -240,17 +239,7 @@ def _update_flag_for_versioning_v1(
240239

241240

242241
def _update_feature_state_value(fsv: FeatureStateValue, value: str, type_: str) -> None:
243-
match type_:
244-
case "string":
245-
fsv.string_value = value
246-
fsv.type = STRING
247-
case "integer":
248-
fsv.integer_value = int(value)
249-
fsv.type = INTEGER
250-
case "boolean":
251-
fsv.boolean_value = value.lower() == "true"
252-
fsv.type = BOOLEAN
253-
242+
fsv.set_value(value, type_)
254243
fsv.save()
255244

256245

api/tests/unit/features/test_unit_features_views.py

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
from features.feature_types import MULTIVARIATE
4848
from features.models import Feature, FeatureSegment, FeatureState
4949
from features.multivariate.models import MultivariateFeatureOption
50-
from features.value_types import BOOLEAN, INTEGER, STRING
50+
from features.value_types import STRING
5151
from features.versioning.models import EnvironmentFeatureVersion
5252
from metadata.models import MetadataModelField
5353
from organisations.models import Organisation, OrganisationRole
@@ -3341,27 +3341,23 @@ def test_list_features_with_feature_state(
33413341
version=100,
33423342
)
33433343
feature_state_value_versioned = feature_state_versioned.feature_state_value
3344-
feature_state_value_versioned.string_value = None
3345-
feature_state_value_versioned.integer_value = 2005
3346-
feature_state_value_versioned.type = INTEGER
3344+
feature_state_value_versioned.set_value("2005", "integer")
33473345
feature_state_value_versioned.save()
33483346

33493347
feature_state2 = feature2.feature_states.filter(environment=environment).first()
33503348
feature_state2.enabled = True
33513349
feature_state2.save()
33523350

33533351
feature_state_value2 = feature_state2.feature_state_value
3354-
feature_state_value2.string_value = None
3355-
feature_state_value2.boolean_value = True
3356-
feature_state_value2.type = BOOLEAN
3352+
feature_state_value2.set_value("true", "boolean")
33573353
feature_state_value2.save()
33583354

33593355
feature_state_value3 = (
33603356
feature3.feature_states.filter(environment=environment)
33613357
.first()
33623358
.feature_state_value
33633359
)
3364-
feature_state_value3.string_value = "present"
3360+
feature_state_value3.set_value("present", "string")
33653361
feature_state_value3.save()
33663362

33673363
# This should be ignored due to identity being set.
@@ -3470,37 +3466,31 @@ def test_list_features_with_filter_by_value_search_string_and_int(
34703466
environment_feature_version1.publish(staff_user)
34713467

34723468
feature_state_value1 = feature_state1.feature_state_value
3473-
feature_state_value1.string_value = None
3474-
feature_state_value1.integer_value = 1945
3475-
feature_state_value1.type = INTEGER
3469+
feature_state_value1.set_value("1945", "integer")
34763470
feature_state_value1.save()
34773471

34783472
feature_state2 = feature2.feature_states.filter(environment=environment).first()
34793473
feature_state2.enabled = True
34803474
feature_state2.save()
34813475

34823476
feature_state_value2 = feature_state2.feature_state_value
3483-
feature_state_value2.string_value = None
3484-
feature_state_value2.boolean_value = True
3485-
feature_state_value2.type = BOOLEAN
3477+
feature_state_value2.set_value("true", "boolean")
34863478
feature_state_value2.save()
34873479

34883480
feature_state_value3 = (
34893481
feature3.feature_states.filter(environment=environment)
34903482
.first()
34913483
.feature_state_value
34923484
)
3493-
feature_state_value3.string_value = "present"
3494-
feature_state_value3.type = STRING
3485+
feature_state_value3.set_value("present", "string")
34953486
feature_state_value3.save()
34963487

34973488
feature_state4 = feature4.feature_states.filter(environment=environment).first()
34983489
feature_state4.enabled = True
34993490
feature_state4.save()
35003491

35013492
feature_state_value4 = feature_state4.feature_state_value
3502-
feature_state_value4.string_value = "year 1945"
3503-
feature_state_value4.type = STRING
3493+
feature_state_value4.set_value("year 1945", "string")
35043494
feature_state_value4.save()
35053495

35063496
base_url = reverse("api-v1:projects:project-features-list", args=[project.id])
@@ -3548,37 +3538,31 @@ def test_list_features_with_filter_by_search_value_boolean(
35483538
feature_state1.save() # type: ignore[union-attr]
35493539

35503540
feature_state_value1 = feature_state1.feature_state_value # type: ignore[union-attr]
3551-
feature_state_value1.string_value = None
3552-
feature_state_value1.integer_value = 1945
3553-
feature_state_value1.type = INTEGER
3541+
feature_state_value1.set_value("1945", "integer")
35543542
feature_state_value1.save()
35553543

35563544
feature_state2 = feature2.feature_states.filter(environment=environment).first()
35573545
feature_state2.enabled = False
35583546
feature_state2.save()
35593547

35603548
feature_state_value2 = feature_state2.feature_state_value
3561-
feature_state_value2.string_value = None
3562-
feature_state_value2.boolean_value = True
3563-
feature_state_value2.type = BOOLEAN
3549+
feature_state_value2.set_value("true", "boolean")
35643550
feature_state_value2.save()
35653551

35663552
feature_state_value3 = (
35673553
feature3.feature_states.filter(environment=environment)
35683554
.first()
35693555
.feature_state_value
35703556
)
3571-
feature_state_value3.string_value = "present"
3572-
feature_state_value3.type = STRING
3557+
feature_state_value3.set_value("present", "string")
35733558
feature_state_value3.save()
35743559

35753560
feature_state4 = feature4.feature_states.filter(environment=environment).first()
35763561
feature_state4.enabled = True
35773562
feature_state4.save()
35783563

35793564
feature_state_value4 = feature_state4.feature_state_value
3580-
feature_state_value4.string_value = "year 1945"
3581-
feature_state_value4.type = STRING
3565+
feature_state_value4.set_value("year 1945", "string")
35823566
feature_state_value4.save()
35833567

35843568
base_url = reverse("api-v1:projects:project-features-list", args=[project.id])
@@ -4094,7 +4078,7 @@ def test_list_features_segment_query_param_with_valid_segment(
40944078
environment=environment,
40954079
enabled=True,
40964080
)
4097-
segment_override.feature_state_value.string_value = "segment_value"
4081+
segment_override.feature_state_value.set_value("segment_value", "string")
40984082
segment_override.feature_state_value.save()
40994083

41004084
base_url = reverse("api-v1:projects:project-features-list", args=[project.id])

0 commit comments

Comments
 (0)