Store initial setup time for Site Kit and modules as meta.

[derweili]

Feature Description

This is a followup for #5853 where showing the auto-update banner is delayed after the initial plugin setup for 10 minutes. Currently there is no way to get the site kit setup time to calculate the time delta. Therefore the auto-update banner (#5853) tries to recognize the initial setup by verifying URL query params. A cacheItem with a 10 minutes lifetime is used to recognize the initial setup for the following 10 minutes.

There are many assumptions in this process, which are unrelated to the to the actual component.

Therefore it would be better, if the setup time was stored in options and available through the datastore. The same applies not only to Site Kit setup but also to the setup time of each module.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• After successfull Site Kit setup, the current time should be stored in options table.
• This setup time should be available through a new selector on the core/site datastore.
• For each module: Adsense, Analytics, Analytics-4, Optimize, Search Console, Tag Manager:
  • After successfull setup of each module, the current time should be stored in the module settings.
  • The modules setup time should be available through a new selector on the modules datastore.
• The Auto-update banner approach used for #5853 should be updated to use this new data.

Implementation Brief

• Using includes/Core/Modules/Modules.php,
  • Within activate_module, update the newly activated module settings by adding setupTime with the value set to the current timestamp.
    • To set the module settings, use $module->get_settings()->merge() function.
• Using each module's settings class, for e.g includes/Modules/AdSense/Settings.php, add the setupTime to the array of settings within the get_default function.
• Add the setupTime setting for the module on the JS side as well for each module when creating the module store. For e.g in assets/js/modules/adsense/datastore/base.js.
  • The getSetupTime selector will automatically be available.
• Similar to includes/Core/Authentication/Verification.php, create includes/Core/Authentication/SetupTime.php to add a new option googlesitekit_setup_time and extending the Setting class instead of User_Setting.
• Within includes/Core/Authentication/Clients/OAuth_Client.php,
  • Set the googlesitekit_setup_time option value to the current timestamp using the new SetupTime class, before the user is redirected to the splash page, with the authentication_success URL param set and there is no redirection url set.
• Using includes/Core/Authentication/Authentication.php,
  • Within the inline_js_base_data method, add setupTime to the returned data, with the value set to a new option googlesitekit_setup_time, which can be obtained by calling $this->options->get method, passing the option name as parameter.
• Using assets/js/googlesitekit/datastore/site/info.js,
  • In the RECEIVE_SITE_INFO reducer, add the new setupTime property to siteInfo.
  • Add a new selector getSetupTime which returns the above value.
• Using assets/js/components/notifications/EnableAutoUpdateBannerNotification.js,
  • Update the logic to set isInitialPluginSetup to use the getSetupTime selector from the core/site datastore so that the component is rendered 10 mins after the setup time.

Test Coverage

• Add JS tests for the new getSetupTime for both, modules data store and the core/site data store.
• Add PHP tests for the new setupTime options for both, modules and core site options.

QA Brief

Changelog entry

[tofumatt]

ACs here are good, thanks for including the module setup dates as well 👍🏻

I did add a note to the ACs to use this new selector in the Auto-update logic; I don't think it warrants a new issue 😄

Moved to IB.

[bethanylang]

@derweili Can you please add a priority to this issue? Thanks!

[techanvil]

Hey @asvinb, the IB is looking good for the AC, but I have some reservations about the AC itself.

• Seeing as the intention is to show the auto-update reminder banner 10 minutes after the user has set up Site Kit, I don't think we should be storing the setup time as a site-wide setting, but rather a user setting. Otherwise, if one user sets up SK, and then a second user does, the second user will reset the timing for the first user as well., Or, if the setting is only stored on the very first setup, the second user may be shown the banner straight after they set up the plugin.
• It's not clear to me why we're adding the timestamp for modules as well. I don't see any plans to use it, and unless there are concrete plans to do so, we shouldn't be adding code that may never be used.

I think this might need to have the AC updated. What's your take on this?

[asvinb]

@techanvil

• I think it makes sense to have the setup time stored as a user setting instead of a site setting.
• Here also I fully agree with you, if we are not going to use the code, we shouldn't be adding them.

Do you want me to update the IB or curious to hear @aaemnnosttv 's thoughts about this.

[techanvil]

Thanks @asvinb. I think it might be worth updating the AC, as well as the IB, seeing as we've caught it at this stage. Interested to hear @aaemnnosttv's thoughts too.

[aaemnnosttv]

Anything user-specific which isn't critical and 1 hr or less is probably better suited as client-side cache rather than a persistent value in the DB. I wouldn't say this is critical, so if the user were to install SK via WP CLI and this doesn't get set and the user sees it right away instead, that seems fine to me.

Moving this back to AC for potential revision.

[techanvil]

Anything user-specific which isn't critical and 1 hr or less is probably better suited as client-side cache rather than a persistent value in the DB. I wouldn't say this is critical, so if the user were to install SK via WP CLI and this doesn't get set and the user sees it right away instead, that seems fine to me.

Moving this back to AC for potential revision.

@aaemnnosttv, while I think this is a perfectly reasonable statement, when reading the Feature Description for this one it's evident that we currently do make use of the client-side cache for this functionality, but we don't have a very good way of identifying when the site has just been setup on the client side. I think this issue is really about adding some server-side state to make this new-site status more reliable, and we might still be relying on client-side caching to determine whether we'd actually actioned the notification.

With that in mind, how do you feel about this one?

[ivonac4]

@aaemnnosttv Seems like this one requires your opinion. Do you have time to take a look anytime soon?