diff --git a/experimenter/experimenter/experiments/api/v5/serializers.py b/experimenter/experimenter/experiments/api/v5/serializers.py index 0ed8dda482..3d0cb17538 100644 --- a/experimenter/experimenter/experiments/api/v5/serializers.py +++ b/experimenter/experimenter/experiments/api/v5/serializers.py @@ -1464,12 +1464,18 @@ def _validate_feature_value( unsupported_version_strs = [ str(v) for v in schemas_in_range.unsupported_versions ] - min_unsupported_version = min(unsupported_version_strs) - max_unsupported_version = max(unsupported_version_strs) + if len(unsupported_version_strs) == 1: + unsupported_versions = unsupported_version_strs[0] + else: + min_unsupported_version = min(unsupported_version_strs) + max_unsupported_version = max(unsupported_version_strs) + unsupported_versions = ( + f"{min_unsupported_version}-{max_unsupported_version}" + ) result.append( NimbusConstants.ERROR_FEATURE_CONFIG_UNSUPPORTED_IN_VERSIONS.format( feature_config=feature_config.name, - versions=f"{min_unsupported_version}-{max_unsupported_version}", + versions=unsupported_versions, ), suppress_errors, ) diff --git a/experimenter/experimenter/experiments/models.py b/experimenter/experimenter/experiments/models.py index 8bdcbc433b..72034f81a5 100644 --- a/experimenter/experimenter/experiments/models.py +++ b/experimenter/experimenter/experiments/models.py @@ -1825,7 +1825,7 @@ def get_versioned_schema_range( min_supported_version = NimbusExperiment.Version.parse(min_supported_version) if min_supported_version > min_version: - if max_version is not None and min_supported_version >= max_version: + if max_version is not None and min_supported_version > max_version: # We will not have any NimbusVerionedSchemas in this # version range. The best we can do is use the # unversioned schema. @@ -1910,6 +1910,23 @@ def between_versions_q( *, prefix: Optional[str] = None, ) -> Q: + """Return a query object that can be used to select all versions between lower and + upper bounds (inclusive). + + Args: + min_version: + The lower bound (inclusive). + + max_version: + The upper bound (inclusive). + + prefix: + An optional prefix to prepend to the field names. This allows the Q object + to be used by related models. + + Returns: + The query object. + """ if prefix is not None: def prefixed(**kwargs: dict[str, Any]): @@ -1921,27 +1938,29 @@ def prefixed(**kwargs: dict[str, Any]): return kwargs # (a, b, c) >= (d, e, f) - # := (a > b) | (a = b & d > e) | (a = b & d = e & c >= f) - # == (a > b) | (a = b & (d > e | (d = e & c >= f))) + # := (a > d) | (a == d & b > e) | (a == d & b == e & c >= f) + # == (a > d) | (a == d & (b > e | (b == e & c >= f) # packaging.version.Version uses major.minor.micro, but # NimbusFeatureVersion uses major.minor.patch (semver). - q = Q(**prefixed(major__gt=min_version.major)) | Q( - **prefixed(major=min_version.major) - ) & ( - Q(**prefixed(minor__gt=min_version.minor)) - | Q(**prefixed(minor=min_version.minor, patch__gte=min_version.micro)) + q = Q(**prefixed(major__gt=min_version.major)) | ( + Q(**prefixed(major=min_version.major)) + & ( + Q(**prefixed(minor__gt=min_version.minor)) + | Q(**prefixed(minor=min_version.minor, patch__gte=min_version.micro)) + ) ) if max_version is not None: - # (a, b, c) < (d, e, f) - # := (a < d) | (a == d & b < e) | (a == d & b == e & c < f) - # == (a < d) | (a == d & (b < e | (b == e & c < f))) - q &= Q(**prefixed(major__lt=max_version.major)) | Q( - **prefixed(major=max_version.major) - ) & ( - Q(**prefixed(minor__lt=max_version.minor)) - | Q(**prefixed(minor=max_version.minor, patch__lt=max_version.micro)) + # (a, b, c) <= (d, e, f) + # := (a < d) | (a == d & b < e) | (a = d & b == e & c <= f) + # == (a < d) | (a == d & (b < e | (b == e & c <= f))) + q &= Q(**prefixed(major__lt=max_version.major)) | ( + Q(**prefixed(major=max_version.major)) + & ( + Q(**prefixed(minor__lt=max_version.minor)) + | Q(**prefixed(minor=max_version.minor, patch__lte=max_version.micro)) + ) ) return q diff --git a/experimenter/experimenter/experiments/tests/api/v5/test_serializers/test_nimbus_ready_for_review_serializer.py b/experimenter/experimenter/experiments/tests/api/v5/test_serializers/test_nimbus_ready_for_review_serializer.py index 83b070500a..2852640d20 100644 --- a/experimenter/experimenter/experiments/tests/api/v5/test_serializers/test_nimbus_ready_for_review_serializer.py +++ b/experimenter/experimenter/experiments/tests/api/v5/test_serializers/test_nimbus_ready_for_review_serializer.py @@ -3205,13 +3205,13 @@ def setUp(self): NimbusExperiment.Version.NO_VERSION, NimbusConstants.ERROR_FEATURE_CONFIG_UNSUPPORTED_IN_VERSIONS.format( feature_config="FEATURE", - versions="121.0.0-121.0.0", + versions="121.0.0", ), as_warning, ), ( NimbusExperiment.Version.FIREFOX_121, - NimbusExperiment.Version.FIREFOX_122, + NimbusExperiment.Version.FIREFOX_121, NimbusConstants.ERROR_FEATURE_CONFIG_UNSUPPORTED_IN_RANGE.format( feature_config="FEATURE", ), @@ -3305,7 +3305,7 @@ def test_validate_feature_versioned_unsupported_versions( ), ( NimbusExperiment.Version.FIREFOX_110, - NimbusExperiment.Version.FIREFOX_121, + NimbusExperiment.Version.FIREFOX_120, [(120, 0, 0)], ), ] @@ -3626,12 +3626,12 @@ def test_validate_feature_versioned_localized(self): NimbusExperiment.Version.NO_VERSION, NimbusConstants.ERROR_FEATURE_CONFIG_UNSUPPORTED_IN_VERSIONS.format( feature_config="FEATURE", - versions="121.0.0-121.0.0", + versions="121.0.0", ), ), ( NimbusExperiment.Version.FIREFOX_121, - NimbusExperiment.Version.FIREFOX_122, + NimbusExperiment.Version.FIREFOX_121, NimbusConstants.ERROR_FEATURE_CONFIG_UNSUPPORTED_IN_RANGE.format( feature_config="FEATURE", ), @@ -3970,8 +3970,8 @@ def test_fml_validate_feature_versioned_range_treatment_branch(self): NimbusExperimentFactory.Lifecycles.CREATED, targeting_config_slug=NimbusExperiment.TargetingConfig.NO_TARGETING, application=NimbusExperiment.Application.FENIX, - firefox_min_version=NimbusExperiment.Version.FIREFOX_120, - firefox_max_version=NimbusExperiment.Version.FIREFOX_120, + firefox_min_version=NimbusExperiment.Version.FIREFOX_121, + firefox_max_version=NimbusExperiment.Version.FIREFOX_121, feature_configs=[feature], ) diff --git a/experimenter/experimenter/experiments/tests/test_models.py b/experimenter/experimenter/experiments/tests/test_models.py index a8d0971e74..f1e69bfb0c 100644 --- a/experimenter/experimenter/experiments/tests/test_models.py +++ b/experimenter/experimenter/experiments/tests/test_models.py @@ -3965,7 +3965,7 @@ def test_schemas_between_versions(self): minor=minor, patch=patch, ) - for major in range(1, 3) + for major in range(1, 4) for minor in range(3) for patch in range(3) ) @@ -3978,7 +3978,7 @@ def test_schemas_between_versions(self): feature_config=feature, version=versions[(major, minor, patch)], ) - for major in range(1, 3) + for major in range(1, 4) for minor in range(3) for patch in range(3) ) @@ -4001,6 +4001,7 @@ def test_schemas_between_versions(self): (2, 0, 1), (2, 0, 2), (2, 1, 0), + (2, 1, 1), ) }, ) @@ -4190,6 +4191,7 @@ def test_get_versioned_schema_range(self): schemas=[ schemas[versions[v]] for v in ( + (123, 1, 0), (123, 0, 0), (122, 1, 0), (122, 0, 0),