Ensure paywall sharing among experiments doesnt reuse their id

[ianrumac]

Changes in this pull request

• Ensure paywall sharing among experiments doesnt reuse their id

Checklist

• All unit tests pass.
• All UI tests pass.
• Demo project builds and runs.
• I added/updated tests or detailed why my change isn't tested.
• I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
• I have run ktlint in the main directory and fixed any issues.
• I have updated the SDK documentation as well as the online docs.
• I have reviewed the contributing guide

[pullfrog]

ℹ️ No correctness problems found in the fix itself — two informational notes inline plus a missing changelog entry.

Reviewed changes — the full diff at f1ffb7d8: moving the two request-scoped fields off the shared cached PaywallView merge and onto request binding, plus the two new test files.

• MergePaywall no longer refreshes experiment / presentationSourceType — PaywallManager.getPaywallView fires this update on every cache hit (including pure queries like getPresentationResult), which is how a second campaign's fetch overwrote the experiment an in-flight presentation would report.
• SetRequest binds them atomically with the request — sets paywall.experiment = experiment ?: state.paywall.experiment and paywall.presentationSourceType = req.presentationSourceType, so PaywallViewState.info can never pair one request's placement with another's experiment.
• Experiment threaded through the presentation pipeline — new PaywallComponents.experiment (from (outcome.triggerResult as? InternalTriggerResult.Paywall)?.experiment) reaches PaywallView.set(...) via InternalGetPaywall and PaywallView.present(...) via presentPaywallView.
• PaywallManagerExperimentIsolationTest — Robolectric regression test replaying the interleaving (B fetches, A fetches the same cached view, A binds, B binds) against a real PaywallManager + real PaywallView.
• PaywallViewStateTest additions — unit-level coverage that MergePaywall leaves both fields alone and that SetRequest binds experiment + source + placement together.

I verified that req.presentationSourceType in SetRequest is the same value the old merge produced (PaywallRequest.presentationSourceType is itself derived from it at GetPaywallVC.kt:52), and enumerated every reader of PaywallView.info / state.paywall.experiment to confirm dropping the fields from the merge introduces no stale-read path — each is provably preceded by that request's own set()/present(). The second Then block of the new isolation test is genuinely discriminating: two of its three assertions fail under the pre-fix merge/bind logic.

ℹ️ No CHANGELOG.md entry for a user-visible analytics fix

This changes which experiment paywall_open and transaction events attribute to a shared paywall, which is exactly the kind of fix integrators read the changelog for. The PR checklist item is unchecked and no entry was added.

Technical details

# Add a `CHANGELOG.md` entry

## Affected sites
- `CHANGELOG.md` — top section is `## 2.8.3`, which matches `version.env`
  (`SUPERWALL_VERSION=2.8.3`), so 2.8.3 is the in-development version and no new
  version heading is needed.

## Required outcome
- One bullet under the existing `## 2.8.3` → `## Fixes` heading (note: `## Fixes`
  is a same-level heading inside each version block in this repo, not `###`),
  describing that paywalls shared between campaigns no longer report another
  campaign's experiment or presentation source.

ℹ️ Nitpicks

• models/paywall/Paywall.kt:240-248 — Paywall.update(paywall) is referenced nowhere in the repo (src/main, src/test, androidTest, superwall-compose, app, example) and still copies presentationSourceType + experiment, i.e. exactly the policy this PR reverses. Worth deleting so it can't be picked up as the "right" way to merge a refreshed paywall.
• PaywallViewState.kt:97-103 — the new comment sits between base.copy(...) and merged.productItems = from.productItems, directly above the pre-existing // Update productItems via setter line, so it reads at a glance as if it's about products. It belongs above the base.copy(...) whose field list it explains.
• PaywallManagerExperimentIsolationTest — PaywallView's mainScope/ioScope live in a private companion backed by real dispatchers and aren't injectable, so the loadWebView() job kicked off during B's cache-miss fetch runs outside the virtual clock and testScheduler.advanceUntilIdle() isn't authoritative for it. Harmless today (every stub it touches is a no-op and IOScope swallows throwables), but worth a comment so a future reader doesn't assume the test is fully deterministic.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes — the delta since the f1ffb7d8 review: both inline notes from that review were taken on board, and the mechanical call-site updates that follow.

• presentationId is now bound per presentation — SetRequest keeps the creation-time id for a view's first binding (state.request == null) and mints a fresh UUID on every re-binding of a cached view, so a shared paywall no longer reports one presentation_id across every presentation.
• experiment is required on the whole seam — the = null default and the ?: state.paywall.experiment fallback are gone from SetRequest, and the requirement propagates up through PaywallView.set/present, presentPaywallView, presentPaywallViewSync and PaywallComponents, so a future call site that forgets the experiment fails to compile.
• MergePaywall's policy comment extended to presentationId and the SetRequest KDoc rewritten to state that it is the single writer of per-presentation metadata.
• New discriminating unit test — setRequest_mintsPresentationIdPerPresentationOfACachedView asserts the first binding keeps the id, MergePaywall leaves it alone, and the second binding mints a different one; the pre-existing "bound without an experiment" case was flipped to assert the stale experiment is not inherited.
• Call sites updated — explicit experiment arguments in PaywallViewDismissTest, PaywallViewTest and PaywallViewStateTest.

I confirmed PaywallViewState.request is never reset to null by any transform (CleanupAfterDestroy, ResetPresentationPreparations, resetTransientPresentationState, destroyed() all leave it), so state.request == null is a sound one-shot "first bind" signal, and that SuperwallPaywallActivity re-creation and the webview crash-retry path call onViewCreated() rather than set/present, so neither re-mints the id mid-presentation. The id also stays coherent with the load events: paywall_response_load_* / paywall_products_load_* are tracked before PaywallRequestManager.updatePaywall mints the id (so they already carry no presentation_id), and loadWebView() only runs on a cache miss, which is exactly the case where the first SetRequest preserves the creation-time id. PaywallComponents.experiment is never actually null — EvaluateRules synthesizes InternalTriggerResult.Paywall(Experiment.presentById(...)) even for the debugger path.

ℹ️ A re-bind can still land on a view that is currently on screen

SetRequest is reached by InternalGetPaywall.kt:41 without any "is this view live?" check: checkNoPaywallAlreadyPresented is wired only into InternalPresentation.kt:29, PublicGetPaywall.kt:105 hardcodes isPaywallPresented = false, and PaywallManager.getPaywallView's cache-hit branch hands back the same instance regardless of activePaywallVcKey. So an embedded getPaywall() for campaign A, while campaign B's presentation of the same paywall is on screen, rebinds that live view. The root cause predates this PR — the merge clobbered experiment the same way, and prepareToDisplay() would additionally detach the live view — but adding presentationId to the rebound set means the visible paywall's remaining events (transaction_*, paywall_close) can now report an id that no paywall_open ever carried. Nothing to fix in this diff; flagging it as the one shape of shared-view collision the new tests don't cover.

Technical details

# Rebinding a live cached view is unguarded

## Affected sites
- `paywall/view/PaywallViewState.kt:135-143` — `SetRequest` mints a new `presentationId` (and
  rewrites `experiment` / `presentationSourceType`) whenever `state.request != null`, with no
  regard for `state.isPresented`.
- `paywall/presentation/get_paywall/InternalGetPaywall.kt:41` — calls `set()` unconditionally on
  whatever view the manager returned.
- `paywall/manager/PaywallManager.kt:79-98` — the cache-hit branch returns the shared instance
  without comparing against `cache.activePaywallVcKey`.
- `paywall/presentation/get_paywall/PublicGetPaywall.kt:105` — the embedded path builds its
  `PresentationRequest` with `isPaywallPresented = false`, so
  `checkNoPaywallAlreadyPresented` (only called from `InternalPresentation.kt:29`) never runs
  for it.

## Required outcome
- Decide whether an embedded `getPaywall()` for a paywall that is currently presented by another
  campaign is supported at all. If it is not, it should fail the request rather than silently
  rebind the live view; if it is, the two presentations need separate view instances.

## Open questions for the human
- Is this collision reachable in practice for your integrators, or does the fact that
  `prepareToDisplay()` detaches the live view mean it is already a known-unsupported usage?
- Does it belong on this PR at all, or as a follow-up issue against `PaywallViewCache`
  sharing semantics?

  ^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}