-
Notifications
You must be signed in to change notification settings - Fork 934
Add delayed premium upsell #22551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add delayed premium upsell #22551
Conversation
…r not to the user
Pull Request Test Coverage Report for Build aaea12fe12c7195d02e0f7ea5fe67d379d3e3227Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
This reverts commit 0f3fb4a.
…e is flagged as unseen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR: 🚧
Also, there are some visual differences in the modal, like
- the background that looks much more red in the implementation than the design
Yoast SEO Premium
is in color in the intro copy of the design- the perks are in bold in the design
so let's involve UX in this at some point.
public function enqueue_assets() { | ||
$user_id = $this->user_helper->get_current_user_id(); | ||
$user_id = $this->user_helper->get_current_user_id(); | ||
$this->introductions_collector->update_metadata_for( $user_id ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory this has the potential to run an extra SELECT and an extra UPDATE query on every admin page load, since it always runs.
However:
- WP caching makes it so that there's no extra SELECT as long as there has been a prior
wp_get_current_user()
call, which it's always the case (aswp_get_current_user()
is being called from a lot of WP functions that we already use in admin calls) - The internal logic of WP's
update_metadata()
is smart enough to check the old value of the metadata and as long as it's a single value and it's the same with what we're about to update it with, it returns early with no UPDATE queries
which means that there will be no extra queries on admin page loads, except the first time that we need to update the meta.
Which means that this is a no-issue, but wanted to leave this comment here for future reference.
Co-authored-by: Leonidas Milosis <[email protected]>
Co-authored-by: Leonidas Milosis <[email protected]>
CR: ✅ |
One issue discovered at acceptance testing, things don't work 100% properly on new installs and we're dependent on not having other introductions for the delayed upsell to be delayed. Steps to reproduce:
The problem lies on the
chunk. Basically, if the second condition is false, we should probably return early with false, not continue with the other code of the function. |
src/introductions/infrastructure/introductions-seen-repository.php
Outdated
Show resolved
Hide resolved
…ons, the upsell would show earlier than expected
There's another issue I discovered, although it exists in the production versions, sort of. It all starts with the old requirement that no introduction should be shown in the FTC. From this PR on, we do that by sending a REST request setting the introductions to not seen again, when on the FTC. If:
then:
because:
We already have that problem in the released versions, but because we have no multiple introductions yet (not until Black Friday comes - until then we only have the AI insights one), it's never triggered. If we release a fix before Black Friday, no user will be affected (unless there's a user that has Yoast installed already, has never visited a Yoast page and goes straight to the FTC during BF, which is a highly unlikely scenario). After careful consideration with @pls78 we think this is not a blocker for this PR either, because the only way we trigger this problem is to have multiple introductions set to be seen but prevented in the FTC. That's unlikely to happen here too because
|
…/wordpress-seo into add-delayed-premium-upsell
Acceptance check is ✅ |
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
[ 'introduction_name' => bool ]
which would flag if that introduction has been seen. Now, introductions that has been seen are represented as an array of the form[ 'is_seen' => bool, 'seen_on' => timestamp ]
.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Test introductions user meta is correctly updated
wp_options
tableoption_value
column whereoption_name
=wpseo
first_activated_on
timestamp with an integer valuewp_usermeta
tablemeta_key
=_yoast_wpseo_introductions
and edit the value of themeta_value
column by copy-pasting the following:wp_usermeta
table, same row wheremeta_key
=_yoast_wpseo_introductions
meta_value
column is now:ai-fix-assessments-upsell
intrduction is marked as not seen with a timestamp of 0ai-generate-titles-and-descriptions-upsell
introduction has the timestamp addedai-brand-insights-pre-launch
introduction has been added with a timestampTest the introduction is shown when the user has activated the plugin more than two weeks ago but only if another introduction has not been seen in the last week
option_value
column whereoption_name
=wpseo
: change thefirst_activated_on
timestamp to something more than two weeks ago (use this site to manipulate time stamps), for example"first_activated_on":i:1755326382;
for August 16th 2025wp_usermeta
table wheremeta_key
=_yoast_wpseo_introductions
: change all the time stamps to something more than a week agoUpsell / Admin / Timed
)Test the introduction is shown when the user has updated the plugin more than two weeks ago but only if another introduction has not been seen in the last week
wp_usermeta
table wheremeta_key
=_yoast_wpseo_introductions
columnoption_value
column whereoption_name
=wpseo
(you will need to unserialize->edit->serialize again to save a proper value)last_updated_on
timestamp to something more than two weeks ago (use this site to manipulate time stamps)previous_version
value to25.8
, like this:"previous_version":s:4:"25.8";
wp_usermeta
table wheremeta_key
=_yoast_wpseo_introductions
: change all the time stamps to something more than a week agoTest the introduction is not shown in the `First Time Configuration` page
wp_usermeta
table wheremeta_key
=_yoast_wpseo_introductions
columnsrc/introductions/application/delayed-premium-upsell.php
, change line 103 fromreturn $this->should_show_after_delay();
toreturn true;
http://YOU_BLOG_URL/wp-admin/admin.php?page=wpseo_dashboard#/first-time-configuration
wp_usermeta
table, same row wheremeta_key
=_yoast_wpseo_introductions
meta_value
now contains the substrings:22:"delayed-premium-upsell";a:2:{s:7:"is_seen";b:0;s:7:"seen_on";i:0;}
Smoke test having the BF modal active too
src\promotions\domain\black-friday-promotion.php
to make sure BF is active:new Time_Interval( \gmmktime( 10, 00, 00, 11, 28, 2024 ), \gmmktime( 10, 00, 00, 12, 3, 2026 ) )
Relevant test scenarios
Test instructions for QA when the code is in the RC
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes #762