[NRT-903] Claim the trial-ended message when showing the upsell - #1375
Merged
Conversation
The trial-ended message is one-shot: reading the server-side property marks it shown. It was serialized by GET /users/me, so any read of user state spent it with no UI involved. The field has been removed from that endpoint, so the add-on now claims the message explicitly at the point it displays it. POST rather than GET because the call has that effect - a retry, prefetch or proxy of a GET would spend a user's only message.
Restores the trial-ended upsell copy, which became unreachable once the server stopped sending show_trial_ended_message in the user details. The claim runs in the background and the dialog is built from its callbacks, so the UI thread is never blocked and the message is only ever spent by the path that displays it. AddonQueryOp replaces QueryOp's failure handler with one that re-raises into the central error handler, so failing soft needs an explicit .failure() handler: a failed claim logs a warning and shows the generic copy rather than an error dialog. That also covers running against a server without the endpoint. Also reports upgrade_cta_viewed and upgrade_cta_clicked to the product metrics collector. The source property carries the copy the user actually saw and is derived once per dialog, so the two events cannot disagree. Property names and values mirror the web upsell's taxonomy so the channels can be reconciled.
Covers the three claim outcomes - owed, not owed, and a failed claim - asserting the title, the source and surface on both events, and that a failure still opens the dialog with the generic copy. Also pins down two things the server cannot observe: that the access-granted path never claims the message, and that dismissing the dialog reports no click.
The parametrized claim_failed case built AnkiHubHTTPError around a Mock. The failure handler logs str(exc), and AnkiHubHTTPError.__str__ reads the response headers and body, which raised TypeError on a Mock. The handler died before it could show the dialog, so the test timed out waiting for it. Matches how the rest of this file builds error responses.
cacoze
reviewed
Aug 20, 2026
cacoze
left a comment
cacoze
left a comment
Contributor
There was a problem hiding this comment.
Two nits from reading this next to the server PR. Neither is a merge blocker — the claim-at-display-time path looks right.
| _track_upsell_event("upgrade_cta_viewed", source) | ||
|
|
||
|
|
||
| def _show_upsell(user_details: dict, parent=aqt.mw) -> None: |
Contributor
There was a problem hiding this comment.
user_details is unused now. I'd drop it from the signature and change the call site to lambda _: _show_upsell(parent).
Keeping the argument makes it look like the copy still comes from GET /users/me, which is the thing this PR is stopping.
|
|
||
| # Third value in the web's `surface` taxonomy, alongside `web_app` (browser) and | ||
| # `anki_webview` (webview hosted in Anki), which are set in _analytics_state.html:17. | ||
| # Note that Smart Search's webview is also served inside the add-on and reports |
Contributor
There was a problem hiding this comment.
The anki_addon vs web_app / anki_webview distinction is worth keeping — that's the contract.
The pointer at _analytics_state.html:17 (and ModalUpsellContent.html:40,47 a few lines below) lives in the Django repo, so the path and line numbers will rot and aren't greppable from here. Naming the properties (source, surface) is enough.
- `_show_upsell` no longer takes `user_details`; the copy comes from the claim endpoint, not from `GET /users/me`. - Drop the Django-repo file/line pointers from the taxonomy comments; name the `source` and `surface` properties instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Safe to merge as https://github.com/AnkiHubSoftware/ankihub/pull/3910 was already deployed |
cacoze
approved these changes
Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
Proposed changes
AnkiHubClient.claim_trial_ended_message()forPOST /users/me/trial-ended-message/claim.show_trial_ended_messagein user details..failure()handler becauseAddonQueryOpre-raises by default.upgrade_cta_viewed/upgrade_cta_clickedwithsourceandsurfacematching the web taxonomy. The native dialog never loads a webview, so the add-on CTA was previously invisible to analytics.How to reproduce
Further comments
Depends on the server endpoint, which is implemented but not yet in production — ship server first. Until then the fail-soft path shows the generic copy.
The
tests/addonsuite was not run locally (segfaults on macOS); CI is the first check on those three tests.