This commit is contained in:
Beth Rennie 2024-08-02 14:48:18 -04:00
Родитель b2b13299a3
Коммит 1fa33d230d
13 изменённых файлов: 245 добавлений и 16 удалений

Просмотреть файл

@ -504,6 +504,7 @@ class NimbusExperimentType(DjangoObjectType):
conclusion_recommendations = graphene.NonNull(
graphene.List(graphene.NonNull(NimbusExperimentConclusionRecommendationEnum))
)
conflicting_live_pref_flips_experiments = graphene.NonNull(graphene.List(graphene.NonNull(graphene.String)))
countries = graphene.List(graphene.NonNull(NimbusCountryType), required=True)
documentation_links = DjangoListField(NimbusDocumentationLinkType)
enrollment_end_date = graphene.DateTime()
@ -597,6 +598,7 @@ class NimbusExperimentType(DjangoObjectType):
"computed_enrollment_days",
"computed_enrollment_end_date",
"conclusion_recommendations",
"conflicting_live_pref_flips_experiments",
"countries",
"documentation_links",
"enrollment_end_date",

Просмотреть файл

@ -405,9 +405,29 @@ class NimbusConstants:
class Version(models.TextChoices):
@staticmethod
def parse(version_str):
def parse(version_str: str) -> version:
return version.parse(version_str.replace("!", "0"))
@classmethod
def parse_if_nonempty(cls, version_str: str) -> Optional[version]:
if version_str == cls.NO_VERSION:
return None
return cls.parse(version_str)
@staticmethod
def version_ranges_overlap(
a: tuple[packaging.version.Version, Optional[packaging.version.Version]],
b: tuple[packaging.version.Version, Optional[packaging.version.Version]],
) -> bool:
(min_a, max_a) = a
(min_b, max_b) = b
return (
(max_b is None or min_a < max_b) and
(max_a is None or min_b < max_a)
)
NO_VERSION = ""
FIREFOX_11 = "11.!"
FIREFOX_12 = "12.!"

Просмотреть файл

@ -1215,6 +1215,83 @@ class NimbusExperiment(NimbusConstants, TargetingConstants, FilterMixin, models.
for rec in self.conclusion_recommendations
]
@property
def conflicting_live_pref_flips_experiments(self):
# This is only applicable to setPref experiments on desktop.
if not self.application_config or self.application != self.Application.DESKTOP:
return []
min_version = self.Version.parse_if_nonempty(self.firefox_min_version)
if min_version is None:
# The minimum version is required to launch Firefox experiments.
# We can report potential conflicts once that has been filled in.
return []
max_version = self.Version.parse_if_nonempty(self.firefox_max_version)
schemas = list(
NimbusVersionedSchema.objects.filter(
NimbusFeatureVersion.objects.between_versions_q(
min_version,
max_version,
prefix="version",
),
feature_config__application=self.Application.DESKTOP,
)
.exclude(set_pref_vars={})
.prefetch_related("feature_config", "version")
)
if not schemas:
return []
prefs = {
pref
for schema in schemas
for pref in schema.set_pref_vars.values()
}
def conflicts(experiment: NimbusExperiment) -> bool:
print(f"checking {experiment.slug} for conflicts")
if not NimbusExperiment.Version.version_ranges_overlap(
(min_version, max_version),
(
NimbusExperiment.Version.parse(experiment.firefox_min_version),
NimbusExperiment.Version.parse_if_nonempty(experiment.firefox_min_version),
)
):
return False
other_feature_values = NimbusBranchFeatureValue.objects.filter(
branch__experiment=self,
feature_config__slug=NimbusConstants.DESKTOP_PREFFLIPS_SLUG,
)
for feature_value in other_feature_values:
# This experiment is live so the feature values must parse.
value = json.loads(feature_value.value)
if prefs & set(value.get("prefs", {}).keys()):
return True
return False
filter_qs = Q(
status=self.Status.LIVE,
application=self.Application.DESKTOP,
feature_config__slug=NimbusConstants.DESKTOP_PREFFLIPS_SLUG
)
if self.channel:
filter_qs &= Q(channel=self.channel)
return [
e.slug
for e in NimbusExperiment.objects.filter(filter_qs)
if conflicts(e)
]
class NimbusBranch(models.Model):
experiment = models.ForeignKey(

Просмотреть файл

@ -2967,11 +2967,9 @@ class TestNimbusReviewSerializerSingleFeature(MockFmlErrorMixin, TestCase):
firefox_max_version=NimbusExperiment.Version.NO_VERSION,
feature_configs=[prefflips_feature],
channel=NimbusExperiment.Channel.NO_CHANNEL,
is_rollout=True,
)
for branch in experiment.treatment_branches:
branch.delete()
feature_value = experiment.reference_branch.feature_values.get(
feature_config=prefflips_feature
)
@ -3010,11 +3008,9 @@ class TestNimbusReviewSerializerSingleFeature(MockFmlErrorMixin, TestCase):
firefox_max_version=NimbusExperiment.Version.NO_VERSION,
feature_configs=[prefflips_feature],
channel=channel,
is_rollout=True,
)
for branch in experiment.treatment_branches:
branch.delete()
feature_value = experiment.reference_branch.feature_values.get(
feature_config=prefflips_feature
)
@ -3060,11 +3056,9 @@ class TestNimbusReviewSerializerSingleFeature(MockFmlErrorMixin, TestCase):
firefox_max_version=NimbusExperiment.Version.NO_VERSION,
feature_configs=[prefflips_feature],
channel=channel,
is_rollout=True,
)
for branch in experiment.treatment_branches:
branch.delete()
feature_value = experiment.reference_branch.feature_values.get(
feature_config=prefflips_feature
)
@ -3115,11 +3109,9 @@ class TestNimbusReviewSerializerSingleFeature(MockFmlErrorMixin, TestCase):
firefox_max_version=NimbusExperiment.Version.NO_VERSION,
channel=NimbusExperiment.Channel.RELEASE,
feature_configs=[prefflips_feature],
is_rollout=True,
)
for branch in experiment.treatment_branches:
branch.delete()
feature_value = experiment.reference_branch.feature_values.get(
feature_config=prefflips_feature
)

Просмотреть файл

@ -602,10 +602,11 @@ class NimbusExperimentFactory(factory.django.DjangoModelFactory):
feature_configs=None,
excluded_experiments=None,
required_experiments=None,
is_rollout=False,
*args,
**kwargs,
):
experiment = super().create(*args, **kwargs)
experiment = super().create(*args, is_rollout=is_rollout, **kwargs)
if branches is not None:
raise factory.FactoryError(
@ -634,7 +635,9 @@ class NimbusExperimentFactory(factory.django.DjangoModelFactory):
experiment=experiment, name="Control"
)
experiment.save()
NimbusBranchFactory.create(experiment=experiment, name="Treatment")
if not is_rollout:
NimbusBranchFactory.create(experiment=experiment, name="Treatment")
return experiment

Просмотреть файл

@ -3909,3 +3909,76 @@ class ApplicationConfigTests(TestCase):
application_config.kinto_collections,
expected_collections,
)
@parameterized.expand(
[
]
)
def test_conflicting_live_pref_flips_experiments(self, rollout_min_version, rollout_max_version, rollout_prefs, experiment_channel, should_conflict):
prefflips_feature = NimbusFeatureConfigFactory.create_desktop_prefflips_feature()
versions = {
major: NimbusFeatureVersion.objects.create(
major=major,
minor=0,
patch=0,
)
for major in (129, 130, 131)
}
setpref_feature = NimbusFeatureConfigFactory.create(
name="test-feature",
slug="test-feature",
schemas=[
NimbusVersionedSchemaFactory.build(
version=versions[129],
set_pref_vars={},
),
NimbusVersionedSchemaFactory.build(
version=versions[130],
set_pref_vars={"var": "foo.bar.baz"},
),
NimbusVersionedSchemaFactory.build(
version=versions[131],
set_pref_vars={"var": "qux.quux.corge"},
),
]
)
live_rollout = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.LifeCycles.CREATED,
slug="live",
application=NimbusExperiments.Application.DESKTOP,
targeting_config_slug=NimbusExperiment.TargetingConfig.NO_TARGETING,
feature_configs=[prefflips_feature],
channel=NimbusExperiment.Channel.RELEASE,
firefox_min_version=min_version,
firefox_max_version=max_version,
is_rollout=True,
)
live_rollout.reference_branch.feature_values.update(value={"prefs": rollout_prefs})
experiment = NimbusExperimentFactory.create_with_lifecycle(
NimbusExperimentFactory.Lifecycles.LIVE,
application=NimbusExperiment.Application.DESKTOP,
targeting_config_slug=NimbusExperiment.TargetingConfig.NO_TARGETING,
feature_configs=[set_pref_feature],
is_rollout=True,
channel=channel,
firefox_min_version=NimbusExperiment.Version.FIREFOX_129,
firefox_max_version=NimbusExperiment.Version.NO_VERSION,
)
if should_conflict:
self.assertEqual(
experiment.conflictingLiveSetPrefExperiments,
[live_rollout.slug],
"experiment should report conflict"
)
else:
self.assertEqual(
experiment.conflictingLiveSetPrefExperiments,
[],
"experiment should not report conflict"
)

Просмотреть файл

@ -74,6 +74,7 @@ type NimbusExperimentType {
computedEndDate: DateTime
computedEnrollmentDays: Int
computedEnrollmentEndDate: DateTime
conflictingLivePrefFlipsExperiments: [String!]!
enrollmentEndDate: DateTime
excludedExperimentsBranches: [NimbusExperimentBranchThroughExcludedType!]!
excludedLiveDeliveries: [String!]!
@ -663,4 +664,4 @@ input ExperimentCloneInput {
parentSlug: String!
name: String!
rolloutBranchSlug: String
}
}

Просмотреть файл

@ -1222,4 +1222,35 @@ describe("PageSummary Warnings", () => {
}
},
);
it.each([
[
[]
],
[
["slug"],
],
[
["slug", "another-slug"],
],
])("displays live prefFlips warnings when appropriate", async (
conflictingLivePrefFlipsExperiments: string[],
) => {
const { mock } = mockExperimentQuery("demo-slug", {
conflictingLivePrefFlipsExperiments
});
const displayWarning = conflictingLivePrefFlipsExperiments.length > 0;
render(<Subject mocks={[mock]} />);
if (displayWarning) {
const warning = screen.getByTestId("live-prefflips");
const slugs = Array.from(warning.querySelectorAll("li"), el => el.textContent?.trim() ?? "");
expect(slugs).toEqual(conflictingLivePrefFlipsExperiments);
} else {
expect(screen.queryByTestId("live-prefflips")).toBeNull();
}
});
});

Просмотреть файл

@ -24,6 +24,7 @@ import {
CHANGELOG_MESSAGES,
EXTERNAL_URLS,
LIFECYCLE_REVIEW_FLOWS,
PREF_FLIPS_WARNINGS,
QA_STATUS_PROPERTIES,
} from "src/lib/constants";
import { ExperimentContext } from "src/lib/contexts";
@ -417,6 +418,7 @@ const WarningList = ({
<Warning
{...{
text: submitError,
key: "submit-error",
testId: "submit-error",
variant: "warning",
}}
@ -430,6 +432,7 @@ const WarningList = ({
<Warning
{...{
text: fieldWarnings.bucketing as SerializerMessage,
key: "bucketing",
testId: "bucketing",
learnMoreLink: EXTERNAL_URLS.BUCKET_WARNING_EXPLANATION,
}}
@ -442,6 +445,7 @@ const WarningList = ({
<Warning
{...{
text: fieldWarnings.firefox_min_version as SerializerMessage,
key: "desktop-min-version",
testId: "desktop-min-version",
variant: "warning",
}}
@ -454,6 +458,7 @@ const WarningList = ({
<Warning
{...{
text: fieldWarnings.pref_rollout_reenroll as SerializerMessage,
key: "rollout-setpref-reenroll",
testId: "rollout-setpref-reenroll",
learnMoreLink: EXTERNAL_URLS.ROLLOUT_SETPREF_REENROLL_EXPLANATION,
}}
@ -468,6 +473,7 @@ const WarningList = ({
{...{
text: AUDIENCE_OVERLAP_WARNINGS.EXCLUDING_EXPERIMENTS_WARNING,
slugs: experiment.excludedLiveDeliveries,
key: "excluding-live-experiments",
testId: "excluding-live-experiments",
variant: "warning",
learnMoreLink: EXTERNAL_URLS.AUDIENCE_OVERLAP_WARNING,
@ -482,6 +488,7 @@ const WarningList = ({
{...{
text: AUDIENCE_OVERLAP_WARNINGS.LIVE_EXPERIMENTS_BUCKET_WARNING,
slugs: experiment.liveExperimentsInNamespace,
key: "live-experiments-in-bucket",
testId: "live-experiments-in-bucket",
variant: "warning",
learnMoreLink: EXTERNAL_URLS.AUDIENCE_OVERLAP_WARNING,
@ -497,12 +504,27 @@ const WarningList = ({
text: AUDIENCE_OVERLAP_WARNINGS.LIVE_MULTIFEATURE_WARNING,
slugs: experiment.featureHasLiveMultifeatureExperiments,
testId: "live-multifeature",
key: "live-multifeature",
variant: "warning",
learnMoreLink: EXTERNAL_URLS.AUDIENCE_OVERLAP_WARNING,
}}
/>,
);
}
if (experiment.conflictingLivePrefFlipsExperiments.length) {
warnings.push(
<Warning
{...{
text: PREF_FLIPS_WARNINGS.LIVE_CONFLICTING_PREF_FLIPS_EXPERIMENTS,
slugs: experiment.conflictingLivePrefFlipsExperiments,
testId: "live-prefflips",
key: "live-prefflips",
variant: "warning",
}}
/>
);
}
}
return <>{warnings}</>;

Просмотреть файл

@ -265,6 +265,8 @@ export const GET_EXPERIMENT_QUERY = gql`
legalSignoff
qaSignoff
vpSignoff
conflictingLivePrefFlipsExperiments
}
}
`;

Просмотреть файл

@ -102,6 +102,10 @@ export const AUDIENCE_OVERLAP_WARNINGS = {
LIVE_MULTIFEATURE_WARNING: `The following multi-feature experiments are LIVE and may reduce the eligible population for your experiment which may result in reduced statistical power and precision. Please check that the configured population proportion has accounted for this: `,
};
export const PREF_FLIPS_WARNINGS = {
LIVE_CONFLICTING_PREF_FLIPS_EXPERIMENTS: "There are live prefFlips experiments that may set prefs that would be set by this experiment. Launching this experiment may cause unenrollments from the following experiments:",
};
export const LIFECYCLE_REVIEW_FLOWS = {
LAUNCH: {
buttonTitle: "Launch Experiment",

Просмотреть файл

@ -686,6 +686,7 @@ export const MOCK_EXPERIMENT: Partial<getExperiment["experimentBySlug"]> = {
qaStatus: NimbusExperimentQAStatusEnum.NOT_SET,
isWeb: false,
subscribers: [],
conflictingLivePrefFlipsExperiments: [],
};
export const MOCK_LIVE_ROLLOUT: Partial<getExperiment["experimentBySlug"]> = {

Просмотреть файл

@ -295,6 +295,7 @@ export interface getExperiment_experimentBySlug {
legalSignoff: boolean;
qaSignoff: boolean;
vpSignoff: boolean;
conflictingLivePrefFlipsExperiments: string[];
}
export interface getExperiment {