From c5ea0b7d5d374627c3b2a66b3cf02dda5d288866 Mon Sep 17 00:00:00 2001 From: ITQ Date: Tue, 24 Feb 2026 07:56:17 +0300 Subject: [PATCH] fix(): project refactoring and minor fixes --- src/backend/apps/decision/services.py | 12 +++++-- src/backend/apps/events/services.py | 36 ++++++++++--------- src/backend/apps/experiments/services.py | 34 ++++++++++++++++++ .../apps/experiments/tests/test_services.py | 6 ++++ .../migrations/0003_alter_featureflag_key.py | 19 ++++++++++ src/backend/apps/flags/models.py | 2 +- src/backend/apps/reports/services.py | 18 ---------- 7 files changed, 90 insertions(+), 37 deletions(-) create mode 100644 src/backend/apps/flags/migrations/0003_alter_featureflag_key.py diff --git a/src/backend/apps/decision/services.py b/src/backend/apps/decision/services.py index bf19ba5..6f8b7d0 100644 --- a/src/backend/apps/decision/services.py +++ b/src/backend/apps/decision/services.py @@ -12,7 +12,12 @@ from apps.conflicts.models import ExperimentConflictDomain from apps.conflicts.services import resolve_domain_conflict from apps.events.models import Decision from apps.events.services import decision_create -from apps.experiments.models import Experiment, ExperimentStatus, Variant +from apps.experiments.models import ( + ACTIVE_STATUSES, + Experiment, + ExperimentStatus, + Variant, +) from apps.experiments.selectors import active_experiment_for_flag from apps.flags.models import FeatureFlag from apps.flags.selectors import feature_flag_get_by_key @@ -33,7 +38,7 @@ MAX_CONCURRENT_EXPERIMENTS = 3 COOLDOWN_DAYS = 7 -def _hash_subject(subject_id: str, experiment_id: str, salt: str) -> float: +def _hash_subject(subject_id: str, experiment_id: str, salt: str) -> Decimal: hash_input = f"{subject_id}:{experiment_id}:{salt}".encode() hash_bytes = hashlib.sha256(hash_input).digest() hash_int = int.from_bytes(hash_bytes[:8], byteorder="big") @@ -112,6 +117,9 @@ def _check_participation_limits( subject_id=subject_id, reason="experiment_assigned", experiment_id__isnull=False, + experiment_id__in=Experiment.objects.filter( + status__in=ACTIVE_STATUSES, + ).values("pk"), ) .exclude(experiment_id=experiment_pk) .values("experiment_id") diff --git a/src/backend/apps/events/services.py b/src/backend/apps/events/services.py index ee2a646..a1a8e13 100644 --- a/src/backend/apps/events/services.py +++ b/src/backend/apps/events/services.py @@ -22,6 +22,7 @@ from apps.events.selectors import ( pending_event_exists, pending_events_for_decision, ) +from config.errors import ConflictError PENDING_TTL_DAYS = 7 @@ -124,23 +125,22 @@ def _process_exposure_event( ) -> None: decision_id = event_data["decision_id"] subject_id = event_data["subject_id"] - timestamp = parse_datetime(event_data["timestamp"]) or timezone.now() + timestamp = parse_datetime(event_data["timestamp"]) + if timestamp is None: + raise ValidationError( + "Field 'timestamp' must be a valid ISO 8601 datetime." + ) decision = decision_get(decision_id) - experiment_id = None - variant_id = None if decision: - experiment_id = decision.experiment_id - variant_id = decision.variant_id - - with suppress(IntegrityError): - Exposure.objects.create( - decision_id=decision_id, - experiment_id=experiment_id, - variant_id=variant_id, - subject_id=subject_id, - timestamp=timestamp, - ) + with suppress(ConflictError): + Exposure.objects.create( + decision_id=decision_id, + experiment_id=decision.experiment_id, + variant_id=decision.variant_id, + subject_id=subject_id, + timestamp=timestamp, + ) Event.objects.create( event_id=event_data["event_id"], @@ -158,7 +158,7 @@ def _process_exposure_event( def _promote_pending_events(decision_id: str) -> None: pending = pending_events_for_decision(decision_id) for pe in pending: - with suppress(IntegrityError): + with suppress(ConflictError): Event.objects.create( event_id=pe.event_id, event_type=pe.event_type, @@ -178,7 +178,11 @@ def _process_conversion_event( ) -> None: decision_id = event_data["decision_id"] subject_id = event_data["subject_id"] - timestamp = parse_datetime(event_data["timestamp"]) or timezone.now() + timestamp = parse_datetime(event_data["timestamp"]) + if timestamp is None: + raise ValidationError( + "Field 'timestamp' must be a valid ISO 8601 datetime." + ) properties = event_data.get("properties", {}) if not event_type_obj.requires_exposure or exposure_exists(decision_id): diff --git a/src/backend/apps/experiments/services.py b/src/backend/apps/experiments/services.py index b2a5594..0961207 100644 --- a/src/backend/apps/experiments/services.py +++ b/src/backend/apps/experiments/services.py @@ -23,8 +23,10 @@ from apps.notifications.services import ( notification_enqueue, ) from apps.reviews.selectors import ( + approver_group_get_by_experimenter, can_user_approve_experimenter, get_min_approvals_for_experimenter, + review_settings_load, ) from apps.users.models import User from config.errors import ForbiddenError @@ -130,6 +132,15 @@ def variant_create( is_control: bool = False, ) -> Variant: ensure_owner_or_admin(experiment, user) + if experiment.status != ExperimentStatus.DRAFT: + raise ValidationError( + { + "experiment": ( + "Variants can only be added while the experiment " + f"is in '{ExperimentStatus.DRAFT}' status." + ) + } + ) validate_value_for_type(value, experiment.flag.value_type) variant = Variant( experiment=experiment, @@ -154,6 +165,15 @@ def variant_update( is_control: bool | None = None, ) -> Variant: ensure_owner_or_admin(variant.experiment, user) + if variant.experiment.status != ExperimentStatus.DRAFT: + raise ValidationError( + { + "experiment": ( + "Variants can only be updated while the experiment " + f"is in '{ExperimentStatus.DRAFT}' status." + ) + } + ) if name is not None: variant.name = name if value is not None: @@ -228,6 +248,20 @@ def _validate_experiment_ready_for_review(experiment: Experiment) -> None: ) } ) + approver_group = approver_group_get_by_experimenter(experiment.owner) + if ( + approver_group is None + and not review_settings_load().allow_any_approver + ): + raise ValidationError( + { + "approvers": ( + "No approvers available for this experiment's owner. " + "Configure an approver group or enable " + "'allow_any_approver'." + ) + } + ) def _validate_no_active_flag_conflict(experiment: Experiment) -> None: diff --git a/src/backend/apps/experiments/tests/test_services.py b/src/backend/apps/experiments/tests/test_services.py index 7532fa5..a46405b 100644 --- a/src/backend/apps/experiments/tests/test_services.py +++ b/src/backend/apps/experiments/tests/test_services.py @@ -157,6 +157,9 @@ class VariantCrudTest(TestCase): class SubmitForReviewTest(TestCase): def test_submit_with_valid_variants(self) -> None: + review_settings_update( + default_min_approvals=1, allow_any_approver=True + ) exp = make_experiment(suffix="_sr") add_two_variants(exp) exp = experiment_submit_for_review(experiment=exp, user=exp.owner) @@ -478,6 +481,9 @@ class LifecycleFlowTest(TestCase): class OwnershipPermissionTest(TestCase): @override def setUp(self) -> None: + review_settings_update( + default_min_approvals=1, allow_any_approver=True + ) self.owner = make_experimenter("_own") self.other = make_experimenter("_oth") self.admin = make_admin("_adm") diff --git a/src/backend/apps/flags/migrations/0003_alter_featureflag_key.py b/src/backend/apps/flags/migrations/0003_alter_featureflag_key.py new file mode 100644 index 0000000..c88e733 --- /dev/null +++ b/src/backend/apps/flags/migrations/0003_alter_featureflag_key.py @@ -0,0 +1,19 @@ +# Generated by Django 5.2.11 on 2026-02-24 04:52 + +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('flags', '0002_alter_featureflag_key'), + ] + + operations = [ + migrations.AlterField( + model_name='featureflag', + name='key', + field=models.CharField(help_text='Unique identifier for the feature flag', max_length=100, unique=True, validators=[django.core.validators.RegexValidator(message='Flag key must follow snake_case, camelCase, or PascalCase.', regex='^[A-Za-z][A-Za-z0-9_]*$')], verbose_name='key'), + ), + ] diff --git a/src/backend/apps/flags/models.py b/src/backend/apps/flags/models.py index eb8af9c..1f97510 100644 --- a/src/backend/apps/flags/models.py +++ b/src/backend/apps/flags/models.py @@ -62,7 +62,7 @@ class FeatureFlag(BaseModel): RegexValidator( regex=FLAG_KEY_PATTERN, message=( - "Event type name must follow snake_case, " + "Flag key must follow snake_case, " "camelCase, or PascalCase." ), ) diff --git a/src/backend/apps/reports/services.py b/src/backend/apps/reports/services.py index 53a4955..62c675c 100644 --- a/src/backend/apps/reports/services.py +++ b/src/backend/apps/reports/services.py @@ -46,24 +46,6 @@ def _count_events( return qs.count() -def _count_unique_subjects( - decision_ids: list[str], - event_type_name: str, - start_date: datetime | None = None, - end_date: datetime | None = None, -) -> int: - qs = Event.objects.filter( - decision_id__in=decision_ids, - event_type__name=event_type_name, - is_attributed=True, - ) - if start_date: - qs = qs.filter(timestamp__gte=start_date) - if end_date: - qs = qs.filter(timestamp__lt=end_date) - return qs.values("subject_id").distinct().count() - - def _average_property( decision_ids: list[str], event_type_name: str,