Commit f3cc3a18 authored by Barret Rennie's avatar Barret Rennie
Browse files

Bug 1836502 - Do not re-enroll in rollouts users opt-out of r=emcminn 

Differential Revision: https://phabricator.services.mozilla.com/D179828
parent 47ba8704

toolkit/components/nimbus/lib/ExperimentManager.sys.mjs

+8 −2
Original line number Diff line number Diff line
@@ -522,7 +522,10 @@ export class _ExperimentManager { 
        );

        this.unenroll(recipe.slug, "bucketing");

        return false;

      } else if (!enrollment.active) {

      } else if (

        !enrollment.active &&

        enrollment.unenrollReason !== "individual-opt-out"

      ) {

        lazy.log.debug(`Re-enrolling in rollout "${recipe.slug}`);

        return !!(await this.enroll(recipe, source, { reenroll: true }));

      }
@@ -600,7 +603,10 @@ export class _ExperimentManager { 
    lazy.TelemetryEnvironment.setExperimentInactive(slug);

    // We also need to set the experiment inactive in the Glean Experiment API

    Services.fog.setExperimentInactive(slug);

    this.store.updateExperiment(slug, { active: false });

    this.store.updateExperiment(slug, {

      active: false,

      unenrollReason: reason,

    });



    lazy.TelemetryEvents.sendEvent("unenroll", TELEMETRY_EVENT_OBJECT, slug, {

      reason,

toolkit/components/nimbus/schemas/NimbusEnrollment.schema.json

+4 −0
Original line number Diff line number Diff line
@@ -203,6 +203,10 @@ 
            }

          ],

          "description": "Per-locale localization substitutions.\n\nThe top level key is the locale (e.g., \"en-US\" or \"fr\"). Each entry is a mapping of string IDs to their localized equivalents.\n\nOnly supported on desktop."

        },

        "unenrollReason": {

          "type": "string",

          "description": "The reason for unenrollment. Only present when the enrollment is inactive."

        }

      },

      "required": [

toolkit/components/nimbus/test/unit/test_ExperimentManager_unenroll.js

+4 −1
Original line number Diff line number Diff line
@@ -384,7 +384,10 @@ add_task(async function test_remove_rollouts() { 
    "Called to set the rollout as !active"

  );

  Assert.ok(

    manager.store.updateExperiment.calledWith(rollout.slug, { active: false }),

    manager.store.updateExperiment.calledWith(rollout.slug, {

      active: false,

      unenrollReason: "some-reason",

    }),

    "Called with expected parameters"

  );

});

toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader_updateRecipes.js

+49 −0
Original line number Diff line number Diff line
@@ -1168,6 +1168,9 @@ add_task(async function test_reenroll_rollout_resized() { 
    "Should unenroll from rollout"

  );



  const enrollment = manager.store.get(rollout.slug);

  Assert.equal(enrollment.unenrollReason, "bucketing");



  rollout.bucketConfig.count = 1000;

  await loader.updateRecipes();
@@ -1177,6 +1180,16 @@ add_task(async function test_reenroll_rollout_resized() { 
    "Should re-enroll in rollout"

  );



  const newEnrollment = manager.store.get(rollout.slug);

  Assert.ok(

    !Object.is(enrollment, newEnrollment),

    "Should have new enrollment object"

  );

  Assert.ok(

    !("unenrollReason" in newEnrollment),

    "New enrollment should not have unenroll reason"

  );



  manager.unenroll(rollout.slug);

  await assertEmptyStore(manager.store, { cleanup: true });

});
@@ -1220,3 +1233,39 @@ add_task(async function test_experiment_reenroll() { 


  await assertEmptyStore(manager.store, { cleanup: true });

});



add_task(async function test_rollout_reenroll_optout() {

  const loader = ExperimentFakes.rsLoader();

  const manager = loader.manager;



  await loader.init();

  await manager.onStartup();

  await manager.store.ready();



  const rollout = ExperimentFakes.recipe("experiment", { isRollout: true });

  rollout.bucketConfig = {

    ...rollout.bucketConfig,

    start: 0,

    count: 1000,

    total: 1000,

  };



  sinon.stub(loader.remoteSettingsClient, "get").resolves([rollout]);

  await loader.updateRecipes();



  Assert.ok(

    manager.store.getRolloutForFeature("testFeature"),

    "Should enroll in rollout"

  );



  manager.unenroll(rollout.slug, "individual-opt-out");



  await loader.updateRecipes();



  Assert.ok(

    !manager.store.getRolloutForFeature("testFeature"),

    "Should not re-enroll in rollout"

  );



  await assertEmptyStore(manager.store, { cleanup: true });

});