Move pageview tracking to OptimizelyProvider

[andrewscfc]

Resolves JIRA:

This PR tries out a suggestion from an Optimizely consultant, I'm not 100% sure if this code structure is clean long term but I'm happy to change it to debug the page view events we lose.

Below is an AI generated PR description verified by me

---

Bug: Optimizely page-views events not sent consistently

The experiment is evaluated client-side via useDecision from @optimizely/react-sdk, which fires a DECISION notification that the listener in withOptimizelyProvider subscribes to.

Previously, track('page-views') was called from the PageViewTracking component, which only rendered when OptimizelyPageMetrics read an active flag key from the optimizelyDecisionStore. The store was only populated when decisionEventDispatched was true — a flag the SDK sets only when it has successfully dispatched an impression event to Optimizely's servers. If that flag was false for any reason, the store was never updated, OptimizelyPageMetrics returned null, and no page-views event fired.

Fix:

• Move track('page-views') into the notification listener in withOptimizelyProvider. The event is now sent directly when a decision is received with decisionEventDispatched: true and an active variationKey, removing the dependency on the optimizelyDecisionStore rendering pipeline.
• Add a URL-keyed idempotency check (trackedPageViewUrls: Set<string>) to ensure track('page-views') fires at most once per page URL. A DECISION notification fires once per decide() call — not once per page load — so without this guard, multiple experiments on the same page would each trigger track(), inflating metrics. Note: track() is not experiment-scoped; Optimizely's backend attributes a single event to all experiments the user is enrolled in, so one call per page is correct. The Set is URL-aware for SPA navigation: a new pathname gets a fresh entry so each page still gets exactly one event.
• Remove the decisionEventDispatched guard from notifyDecision. notifyDecision now runs on every eligible decision (active flagKey and variationKey !== 'off'), which is safe because notifyDecision already has its own idempotency check (activatedExperiments.has(flagKey)).
• Remove the newswb_ws_topic_discovery_module special case, which allowed tracking even when variationKey was 'off'. The listener now applies a single consistent rule.
• PageViewTracking is updated accordingly — it no longer tracks page-views directly and instead focuses solely on optional visit event tracking.

Testing

1. List the steps required to test this PR.

Useful Links

• Coding Standards
• Repository use guidelines
• Add Links to useful resources related to this PR if applicable.

[Copilot]

Pull request overview

This PR fixes inconsistent Optimizely view-event tracking by ensuring Simorgh records experiment activation on every Optimizely DECISION notification, rather than only when Optimizely reports that it successfully dispatched an impression event.

Changes:

• Removes the decisionEventDispatched property from the DECISION notification payload typing.
• Removes the decisionEventDispatched gate so notifyDecision(flagKey) runs whenever an eligible decision is received.

Comments suppressed due to low confidence (1)

src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.tsx:57

• The decision notification listener change isn’t covered by the existing withOptimizelyProvider tests. Given this PR’s goal (ensuring tracking renders even when Optimizely’s decisionEventDispatched is false), it would be valuable to add a unit test that mocks notificationCenter.addNotificationListener, invokes the registered callback with a DECISION payload where decisionInfo.flagKey is set and decisionInfo.decisionEventDispatched is false, and asserts notifyDecision(flagKey) is still called exactly once (and is idempotent on subsequent calls).

optimizely?.notificationCenter?.addNotificationListener(
  enums.NOTIFICATION_TYPES.DECISION,
  (
    notification: ListenerPayload & {
      decisionInfo?: {
        flagKey?: string;
        variationKey?: string;
      };
    },
  ) => {
    const flagKey = notification.decisionInfo?.flagKey;
    const variationKey = notification.decisionInfo?.variationKey;

    if (
      flagKey &&
      (variationKey !== 'off' || flagKey === 'newswb_ws_topic_discovery_module')
    ) {
      notifyDecision(flagKey);
    }

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[andrewscfc]

I've addressed this point in 2d7b39a

Following some AI prompting here's a bit more info on why this fix appears to be correct from the AI; let me know if you think this incorrect, it made sense to me 🤔 :

We've addressed this with a URL-keyed idempotency check — a module-level Set<string> (trackedPageViewUrls) that ensures track('page-views') fires at most once per page URL, regardless of how many experiments evaluate on that page.

One thing worth clarifying for context: track() is not experiment-scoped. It fires a generic conversion event and Optimizely's backend handles attribution — it looks up every experiment the user is enrolled in and attributes that single event to all of them. So calling it once per page is the correct behaviour; calling it once per DECISION (the previous behaviour) would have inflated metrics by crediting each experiment with N pageviews for N experiments on the same page.

The Set is also URL-aware for SPA navigation: a new pathname gets a fresh entry, so each page load still gets exactly one page-views event.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[andrewscfc]

Addressed in 203aaa5

[andrewscfc]

Can you help me with this @LilyL0u ? Does the visit really need to be sent before the pageview? Obviously that poses problems for this PR as a whole if so

[LilyL0u]

Does this mean - doesn't track any events when the user is not in an experiment? Not sure what is meant by 'when visit tracking is disabled' and what is disabling it here.

[LilyL0u]

Oh is it for the event 'visit' that is being sent?

[andrewscfc]

Suggested change

	it('should call onReady but not track any events when visit tracking is disabled', async () => {
	it('should not track any events when visit tracking is disabled', async () => {

yeah, I guess it checks the visit event isn't sent when visit tracking is disabled. Previously this will have sent view tracking without a visit.

Is this wording better?