Repository navigation
WOOA7S-2038: Premium Analytics: Show a notice when the site nears or passes its views limit - #53401
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Premium Analytics plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 2 files.
2 files are newly checked for coverage.
|
670a47a to
202082a
Compare
chihsuan
left a comment
There was a problem hiding this comment.
Nice work bringing the limit notice over to v2! @Nikschavan I tested the near and over states, and both look great. 👍 I left a few small inline notes.
…ews limit Carries over the Stats tier upgrade notice: an info notice at 90% of the plan's billable views limit, which the reader can postpone for a week (shared with Stats through the tier_upgrade notice), and a warning past the limit that cannot be dismissed. The plan-usage widget and the notice now share one Stats purchase URL helper.
…plan limit notice The view event fires once per page load and status, since every section tab mounts its own copy of the notice. Upgrade clicks are recorded here because the Stats purchase page only tracks the from values it knows.
…s sections The dismissal now lands in the shared notices cache before the request finishes, so a section that mounts during the save reads it as dismissed. A failed save restores the cache and shows an error snackbar, so the notice is not left hidden although nothing was saved. The tests now sit on the exact 90% and 100% boundaries.
202082a to
a069dff
Compare
The notice builds its link with getPlanUpgradeUrl(), which now takes the from value, so widgets-toolkit no longer exports a second builder. The dismissal error is reported from the mutation hook's own onError, so it still shows when the reader switches section before the request fails. The stage renders the notice only outside customize mode, which replaces the enabled prop.
Views past the limit are still counted, so the notice no longer says they are not. It carries the wording of the Stats tier upgrade notice instead, and drops the usage sentence.
The plan limit notice reads plan usage on every dashboard load, and the helper blocked that request, so the proxy answered 500 and the specs failed. Fixtures now sit in one table of API path to builder, and usage answers a plan with no views limit.
The fixture table passed the query to every builder through call_user_func(), and Phan rejects that for the plan usage builder, which takes none. A switch on the path calls each builder directly.
dognose24
left a comment
There was a problem hiding this comment.
Approving. I traced the data path and it holds: jetpack-stats/usage through the allowlisted jetpack-stats prefix, the 90% and 100% thresholds matching Calypso's use-plan-usage-query (8999 stays quiet, 9000 shows), the Simple and VIP gates, the 7-day postpone through the shared notices endpoint with the optimistic update keyed on the right query prefix, no notice while the usage or notices request is unresolved, and the Settings card and the notice now sharing getPlanUpgradeUrl(). Changelog set is right, with Simple correctly left out.
Two notes below, neither blocking.
The empty Traffic widgets now ask for the all-time views in /stats, and the helper blocked that path because it only matched paths under /stats/. The proxy returned 500 and the dashboard fixture failed the specs. The helper now looks fixtures up per path, in the same shape as #53401, and answers the summary with views so the widgets keep the period text.
…stponement Past the limit the notice stays for the cycle, as the design asks, so it does not read the tier_upgrade postponement that Stats v1 can set from either state. The comment now says so, since the two dashboards can disagree for up to a week.
The empty Traffic widgets now ask for the all-time views in /stats, and the helper blocked that path because it only matched paths under /stats/. The proxy returned 500 and the dashboard fixture failed the specs. The helper now looks fixtures up per path, in the same shape as #53401, and answers the summary with views so the widgets keep the period text.
The empty Traffic widgets now ask for the all-time views in /stats, and the helper blocked that path because it only matched paths under /stats/. The proxy returned 500 and the dashboard fixture failed the specs. The helper now looks fixtures up per path, in the same shape as #53401, and answers the summary with views so the widgets keep the period text.
Fixes WOOA7S-2038
Proposed changes
views_limitfor "approaching" and 100% for "over".infonotice that can be dismissed. Dismissing it postpones thetier_upgradenotice for a week through the existing notices endpoint, the same way Stats does, so the two dashboards share the state.warningand cannot be dismissed.views_limitisnull) never see it.getPlanUpgradeUrl(), the same builder the plan usage card on the Settings tab uses. It now takes thefromvalue, so the notice reports its own source to the checkout.Related product discussion/links
Does this pull request change what data or activity we track or use?
Yes. The notice records three new Tracks events:
jetpack_premium_analytics_plan_limit_notice_view(once per page load, withstatusset tonearorover),jetpack_premium_analytics_plan_limit_notice_upgrade_click(withstatus) andjetpack_premium_analytics_plan_limit_notice_dismiss. The upgrade link also keeps its ownfromvalue, so purchase completion can be tied to it later.Testing instructions
The notice needs a site with a views limit. The easiest way to test is to fake the plan usage reply with the mu-plugin attached to WOOA7S-2038.
wp-content/mu-plugins/. It replaces the reply of the plan usage endpoint while a cookie is set, and the cookie is set from a URL parameter.wp-admin/admin.php?page=jetpack-premium-analytics-wp-admin&pa_fake_usage=near. A blue notice says "You are nearing your monthly limit", with an "Upgrade now" link and a close button.from=jetpack-premium-analytics-plan-limit-notice.pa_fake_usage=over. A yellow notice says "You have reached your views limit", and there is no close button.pa_fake_usage=nearand close the notice, then reload: it stays hidden. This postpones the realtier_upgradenotice for the site for a week, in Stats v1 too.pa_fake_usage=offto see the real data again. A site under 90% of its limit, or without a limit, shows no notice.I tested both states on a local site with that mu-plugin. I did not dismiss the notice in the browser, because that postpones the real notice for a week; the unit tests cover the dismissal.