site-kit-wp
site-kit-wp copied to clipboard
Encourage users to enable auto-updates for Site Kit
Feature Description
As a plugin which is very actively maintained and updated regularly, it makes sense to always keep Site Kit up to date on the latest version. While some users likely already have this enabled for Site Kit, this would be an easy thing we could encourage users to enable, e.g. right after activating the plugin in the success banner. This should help users to enjoy the latest features and fixes for Site Kit with less manual intervention on their part.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- If the new "Enable auto-updates" link is available on the Plugins page, show a Banner Notification on the Dashboard with a CTA pointing to the same link.
- Title: Keep Site Kit up-to-date
- Description: Turn on auto-updates so you always have the latest version of Site Kit. We constantly introduce new features to help you get the insights you need to be successful on the web.
- Primary CTA: Enable auto-updates
- Secondary Link: Maybe later
- Show the banner only if the above link is available on the plugins page, i.e. at least the following conditions should be met:
- The auto-updates feature is only available for WordPress 5.5 and above.
- The user should also have the
update_plugins
core capability. -
Auto updates for plugins should be enabled - see
wp_is_auto_update_enabled_for_type()
and so should the new user interface elements - seeDisable the auto-update user interface elements
.
- For users who had already set up Site Kit, render the new Banner Notification if the above criteria is met.
- For users who have not set up Site Kit, render the new Banner Notification after the
SetupSuccessBannerNotification
. - Existing users who already have Site Kit setup but do not have auto-updates enabled for Site Kit should also see the banner.
- Clicking on the Primary CTA will call the same WordPress action which is triggered by the new "Enable auto-updates" toggle on the Plugins page. The notification should then be dismissed.
- Clicking on the Secondary Link will dismiss the notification forever.
Implementation Brief
Test Coverage
QA Brief
Changelog entry
@aaemnnosttv Have added some preliminary ACs here based on our discussion. The notification text, time after setup to display the notification and whether to re-show the notification after dismissal need to be finalised.
c.c. @marrrmarrr @felixarntz
Thanks @jimmymadon – this is almost there I think; a few points to iterate on:
"SiteKit" -> "Site Kit" 😉
On versions of WordPress where the new "Enable auto-updates" link is available
Let's mention the specific minimum version of WP we need for this. Also, we should only prompt users who have the necessary core capability to make this change as well.
Apart from that, I wonder if a dashboard notification is the best form for this. Maybe we should add it to the splash page next to the telemetry opt-in? Thoughts @marrrmarrr ?
@aaemnnosttv Dug a bit deeper into this one and have updated the ACs with links to relevant docs.
The only purpose of keeping this a Banner Notification is that we want to show this to older users who have already connected/set up Site Kit. Maybe we can add a delay for new users (similar to the delay in the new TwG Supporter Wall widget prompt notification) so they don't get multiple stacked notifications after setup which can be overwhelming.
Thanks @jimmymadon
- The Banner Notification should be shown <x time?> after setting up the plugin.
I don't think we need to add a particular delay to it. User's already won't see it as the main thing after connecting immediately as there would be other higher-priority banners they would see – at least the success/congrats banner.
Why not also add a CTA to the ActivationNotice? The user should have just activated the plugin so it would be a reasonable time to suggest it I think. I'm not saying instead of the banner here, but as an additional message.
- Clicking on the Secondary Link will dismiss the notification <forever?>.
Yes, we shouldn't nag them on a regular basis about it. It's just a nice to have 👍
We constantly introduce new features to help you get the insights you need to be successful on the web.
We can always revise the wording while we're in motion, just thinking this could be simplified a little bit. Perhaps: We're constantly adding new features and fixes to help you be successful on the web.
Apart from that, I feel like there might be a little more we need to consider after reviewing the intro blog post you shared. I.e. users could have auto-updates enabled for all plugins ("Blanket auto-update opt-in") or interface elements disabled. The AC can be a bit loose about this as high level criteria but we should be sure this is covered in the IB so it might be worth noting somewhere.
@aaemnnosttv Thanks for clarifying some of those questions.
Why not also add a CTA to the ActivationNotice? The user should have just activated the plugin so it would be a reasonable time to suggest it I think. I'm not saying instead of the banner here, but as an additional message.
Did you mean to add a Primary CTA
button to SetupSuccessBannerNotification
(instead of the Got it!
) dismiss button? If so, we will have to change the script of this banner notification. Also, we will have to remove the Got it!
dismiss button to Maybe Later
and clicking on it would mean not showing the new Auto-update Banner Notification. Flow-wise, this is better as it reduces one banner notification/click for newly setup users. Implementation-wise, this is slightly more complicated.
Did you mean to add a
Primary CTA
button toSetupSuccessBannerNotification
(instead of theGot it!
) dismiss button? If so, we will have to change the script of this banner notification.
Ah no, I meant as part of ActivationApp
which shows on the plugins page after activating.
@marrrmarrr @felixarntz In @aaemnnosttv's absence, could one of you please have a look if the ACs here look alright? Have incorporated all of Evan's suggestions and our review of this issue a while back. Thanks!
@aaemnnosttv Based on our AC sync today, we decided to make this as simple as possible and only stick to creating the banner which would be shown soon after someone activates and sets up the plugin. I have created another issue #6092 for modifying the ActivationApp
notice.
@aaemnnosttv @jimmymadon Adding to the above that the same banner should be used for existing users. It shouldn't show right away when having completed the initial setup flow, but any time after, e.g. a few minutes later or so. For users that already had configured the plugin, it can show at any point.
@felixarntz I am guessing your issue with showing the banner immediately after setup is that there will potentially be three banners back-to-back after setup (SetupSuccessBannerNotification
, ZeroData
and the EnableAutoUpdates
)? In that case, should we explore adding a CTA to the SetupSuccessBannerNotification
and only show the new banner to existing users? The alternative as you suggest is creating a time delay but that will involve yet another transient.
@jimmymadon I think we can use a client-side cache value to manage the delay as it's a rather short duration. I imagine this could work similar to how we handle the feature tour cooldown.
Hey @jimmymadon, a minor point but looking at the AC, the "Maybe later" dismiss text is a bit at odds with the last AC point:
- Clicking on the Secondary Link should dismiss the notification forever.
I'm wondering if the text should be "Dismiss" or the dismissal should expire. What do you think?
Hi @sashadoes, nice work on the IB so far. A few points:
- It might be worth clarifying that
wp_is_auto_update_enabled_for_type
should only be called if it exists, which it obviously won't for WP versions below 5.5. - The example
onDismiss
prop you linked to should not be needed. Instead just passingBannerNotification
the propsisDismissable: true
andexpires: 0
should suffice. However there is a related question about the AC here which I have asked @jimmymadon above so it's worth holding off for an answer on that one before finalising this aspect of the IB.- Although this specific link should not be needed, in general please remember to use GitHub permalinks when linking to code to make sure the link doesn't go out of date.
- I am not sure about the following point, which could use some clarification. Could you elaborate further on this, please?
- Use
setItem
andgetItem
fromapi/cache.js
to create a delay before displaying soEnableAutoUpdateBannerNotification
will appear in next render
- Finally, on the testing front,
- I'd suggest adding some PHPUnit test coverage for the additions to
Authentication.php
. - Also, just on a conceptual point, it's worth noting that the approach to JS component testing is more along the lines of integration testing than unit testing, using React Testing Library. In general we aim for a more integrated approach to testing as you will also see in PHPUnit tests. We do have some tests which are unit tests though where there are no externalities or simply because that's the only way to test something. As a result we have a bit of a mixture of integration and unit style JS tests. I will often just refer to them as "Jest tests" because of this ambiguity!
- I'd suggest adding some PHPUnit test coverage for the additions to
@techanvil That is a good point. The text probably should just be "Dismiss" in this case. The "Maybe later" could also be applicable as the user still has the ability to switch on Auto Updates, albeit from the WordPress Plugins screen. But I would lean towards "Dismiss" as that implies "we won't be bugging you about this later".
Hi @tofumatt and @jimmymadon. Thanks for your assistance.
@tofumatt, I added some changes to IB as per your review. For the point:
- Use
setItem
andgetItem
fromapi/cache.js
to create a delay before displaying soEnableAutoUpdateBannerNotification
will appear in the next render
I thought we can cache a flag for the initial load, so EnableAutoUpdateBannerNotification
will not appear on the initial load as was considered in this comment -> https://github.com/google/site-kit-wp/issues/5853#issuecomment-1297496050
The idea for the implementation I caught from this comment -> https://github.com/google/site-kit-wp/issues/5853#issuecomment-1312265751
Please let me know if there are any others better alternatives... Thank you.
Hey @sashadoes, thanks for clarifying that.
Reading the comment you linked to, it looks like this AC point is a little out of date. It should also be noted that it doesn't really convey the intended behaviour of the notification that the point would provide if implemented.
- For users who have not set up Site Kit, render the new Banner Notification after the
SetupSuccessBannerNotification
.
With that in mind I'd suggest going into a little more detail in the IB for this. Something like the following, within the definition for EnableAutoUpdateBannerNotification
. Note that the query param notification=authentication_success
/ slug
conditions are evident in assets/js/components/notifications/SetupSuccessBannerNotification.js
.
- In order to ensure a delay for showing the notification after the initial setup (see https://github.com/google/site-kit-wp/issues/5853#issuecomment-1297496050):
- Check the condition for showing the initial setup notification by checking the query param
notification=authentication_success
is present while the query paramslug
is not present. - If these conditions are met, use
setItem
with attl
of600
seconds (10 minutes) to set a flag, and returnnull
. - Otherwise, check if the flag is set with
getItem
, and if it's not set, returnnull
.
- Check the condition for showing the initial setup notification by checking the query param
@techanvil Apologies - I guess we may have moved this to IB while still discussing some AC points. Have updated that AC point again. Thanks for flagging.
c.c. @sashadoes
No worries, and thanks @jimmymadon!
Thanks @sashadoes!
IB :white_check_mark:
QA Update: ✅
On a existing Site Kit setup:
- Enabling auto updates for Site Kit, the auto update notification does not show up.
- Disabling auto updates for Site Kit, the new auto update notification shows up.
- Clicking the Enable auto-updates CTA Button, the banner text changes to a success notification with only a "Dismiss" button after clicking "Enable auto-updates"
- Clicking "Dismiss" the Banner disappears.
- Confirmed the auto-updates are enabled for Site Kit.
On an new Site:
- When auto updates are disabled and Site Kit is setup.
- After being redirected from Site Kit Service to Site Kit Dashboard, the new notification banner does not show up.
- After flushing session storage, reload Site Kit Dashboard, the auto update banner shows up again.
- When I activated the plugin to disable auto update, the auto updates notification does not show up when plugin auto updates are disabled.
Additional QA: Tested the above for a multisite and the checks above pass. The banner appears on the multisites when auto updates is disabled.
Screencasts
https://user-images.githubusercontent.com/73545194/214072860-a49789ed-0168-4ec0-821b-1b2396a092d2.mp4
https://user-images.githubusercontent.com/73545194/214072939-9f57a10b-9e1a-435c-8a49-3376e05d4adf.mp4
https://user-images.githubusercontent.com/73545194/214072952-1fdbe071-e75a-4f90-9d42-e66e2b2a884b.mp4
QA notes for future reference:
For InstaWp sites the auto updates are disabled by default so you have to alter code in the wp-config file. Search for define( 'AUTOMATIC_UPDATER_DISABLED', true );
and change it to define( 'AUTOMATIC_UPDATER_DISABLED', false );
so that the options appear in the plugins page to enable/disable auto updates.
The plugin I used to disable auto update was Disable All WordPress Updates
which can be found here.