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

Bug 1754220 - Update ASRouter messages when force-enrolling in an experiment r=dmose

parent ab022b52
Loading
Loading
Loading
Loading
+31 −4
Original line number Diff line number Diff line
@@ -91,6 +91,7 @@ const JEXL_PROVIDER_CACHE = new Set(["snippets"]);

// To observe the app locale change notification.
const TOPIC_INTL_LOCALE_CHANGED = "intl:app-locales-changed";
const TOPIC_EXPERIMENT_FORCE_ENROLLED = "nimbus:force-enroll";
// To observe the pref that controls if ASRouter should use the remote Fluent files for l10n.
const USE_REMOTE_L10N_PREF =
  "browser.newtabpage.activity-stream.asrouter.useRemoteL10n";
@@ -540,6 +541,9 @@ class _ASRouter {
    this.isUnblockedMessage = this.isUnblockedMessage.bind(this);
    this.unblockAll = this.unblockAll.bind(this);
    this.forceWNPanel = this.forceWNPanel.bind(this);
    this._onExperimentForceEnrolled = this._onExperimentForceEnrolled.bind(
      this
    );
    Services.telemetry.setEventRecordingEnabled(REACH_EVENT_CATEGORY, true);
  }

@@ -748,10 +752,14 @@ class _ASRouter {
  /**
   * loadMessagesFromAllProviders - Loads messages from all providers if they require updates.
   *                                Checks the .lastUpdated field on each provider to see if updates are needed
   * @param toUpdate  An optional list of providers to update. This overrides
   *                  the checks to determine which providers to update.
   * @memberof _ASRouter
   */
  async loadMessagesFromAllProviders() {
    const needsUpdate = this.state.providers.filter(provider =>
  async loadMessagesFromAllProviders(toUpdate = undefined) {
    const needsUpdate = Array.isArray(toUpdate)
      ? toUpdate
      : this.state.providers.filter(provider =>
          MessageLoaderUtils.shouldProviderUpdate(provider)
        );
    await this.loadAllMessageGroups();
@@ -917,6 +925,10 @@ class _ASRouter {

    SpecialMessageActions.blockMessageById = this.blockMessageById;
    Services.obs.addObserver(this._onLocaleChanged, TOPIC_INTL_LOCALE_CHANGED);
    Services.obs.addObserver(
      this._onExperimentForceEnrolled,
      TOPIC_EXPERIMENT_FORCE_ENROLLED
    );
    Services.prefs.addObserver(USE_REMOTE_L10N_PREF, this);
    // sets .initialized to true and resolves .waitForInitialized promise
    this._finishInitializing();
@@ -946,6 +958,10 @@ class _ASRouter {
      this._onLocaleChanged,
      TOPIC_INTL_LOCALE_CHANGED
    );
    Services.obs.removeObserver(
      this._onExperimentForceEnrolled,
      TOPIC_EXPERIMENT_FORCE_ENROLLED
    );
    Services.prefs.removeObserver(USE_REMOTE_L10N_PREF, this);
    // If we added any CFR recommendations, they need to be removed
    CFRPageActions.clearRecommendations();
@@ -1736,6 +1752,17 @@ class _ASRouter {
    // Removing the button is enough to close the panel.
    await ToolbarPanelHub._hideToolbarButton(win);
  }

  async _onExperimentForceEnrolled(subject, topic, data) {
    const experimentProvider = this.state.providers.find(
      p => p.id === "messaging-experiments"
    );
    if (!experimentProvider.enabled) {
      return;
    }

    await this.loadMessagesFromAllProviders([experimentProvider]);
  }
}
this._ASRouter = _ASRouter;

+44 −0
Original line number Diff line number Diff line
const { BrowserTestUtils } = ChromeUtils.import(
  "resource://testing-common/BrowserTestUtils.jsm"
);
const { RemoteSettings } = ChromeUtils.import(
  "resource://services-settings/remote-settings.js"
);
@@ -13,6 +16,9 @@ const { ExperimentAPI } = ChromeUtils.import(
const { ExperimentFakes, ExperimentTestUtils } = ChromeUtils.import(
  "resource://testing-common/NimbusTestUtils.jsm"
);
const { ExperimentManager } = ChromeUtils.import(
  "resource://nimbus/lib/ExperimentManager.jsm"
);
const { TelemetryFeed } = ChromeUtils.import(
  "resource://activity-stream/lib/TelemetryFeed.jsm"
);
@@ -335,3 +341,41 @@ add_task(async function test_exposure_ping_legacy() {
  exposureSpy.restore();
  await cleanup();
});

add_task(async function test_forceEnrollUpdatesMessages() {
  const experiment = await getCFRExperiment();

  await setup(experiment);
  await SpecialPowers.pushPrefEnv({
    set: [["nimbus.debug", true]],
  });
  registerCleanupFunction(async () => {
    await ExperimentManager.unenroll(`optin-${experiment.slug}`, "cleanup");
    await SpecialPowers.popPrefEnv();
    await cleanup();
  });

  Assert.equal(
    ASRouter.state.messages.filter(m => m.id === "xman_test_message").length,
    0,
    "Experiment message should not be found until we opt in"
  );

  await RemoteSettingsExperimentLoader.optInToExperiment({
    slug: experiment.slug,
    branch: experiment.branches[0].slug,
  });

  await BrowserTestUtils.waitForCondition(
    () =>
      !!ASRouter.state.messages.filter(m => m.id === "xman_test_message")
        .length,
    "waiting for ASRouter to update messages"
  );

  Assert.equal(
    ASRouter.state.messages.filter(m => m.id === "xman_test_message").length,
    1,
    "Experiment message should be found after opt in"
  );
});
+5 −1
Original line number Diff line number Diff line
@@ -352,7 +352,7 @@ class _ExperimentManager {

    recipe.userFacingName = `${recipe.userFacingName} - Forced enrollment`;

    return this._enroll(
    const experiment = this._enroll(
      {
        ...recipe,
        slug: `optin-${recipe.slug}`,
@@ -361,6 +361,10 @@ class _ExperimentManager {
      source,
      { force: true }
    );

    Services.obs.notifyObservers(null, "nimbus:force-enroll");

    return experiment;
  }

  /**