Open aaemnnosttv opened 1 month ago
Hello @aaemnnosttv
fix: 9141
- Replace current timestamp to use
WEEK_IN_SECONDS
instead of a day
@zutigrm, this will be applied only to the new SK installations, everyone else who has already had that cron job active won't switch to the weekly schedule. We need to unregister the existing schedule and re-register it again if the existing schedule is daily.
- Check if
Ads
module is connected to return early. You can usegooglesitekit_is_module_connected
filter for the check
This should happen in the cron job hook itself.
@eugene-manuilov
Replace current timestamp to use WEEK_IN_SECONDS instead of a day @zutigrm, this will be applied only to the new SK installations, everyone else who has already had that cron job active won't switch to the weekly schedule. We need to unregister the existing schedule and re-register it again if the existing schedule is daily.
It won't be necessary, because we only schedule this as a single non-repeating event each time. So if they have something scheduled for that day, as soon as that last event is executed, starting from next one, they will all be scheduled on a weekly basis.
Check if Ads module is connected to return early. You can use googlesitekit_is_module_connected filter for the check This should happen in the cron job hook itself.
Thanks, IB updated
Replace current timestamp to use WEEK_IN_SECONDS instead of a day @zutigrm, this will be applied only to the new SK installations, everyone else who has already had that cron job active won't switch to the weekly schedule. We need to unregister the existing schedule and re-register it again if the existing schedule is daily.
It won't be necessary, because we only schedule this as a single non-repeating event each time. So if they have something scheduled for that day, as soon as that last event is executed, starting from next one, they will all be scheduled on a weekly basis.
Ah... Yes, you are correct. 👍
IB ✔️
Verified:
Followed the steps in the QAB:
googlesitekit_cron_synchronize_ads_linked_data
it appears after refreshing the dashboard fully with the next execution set to 7 days
Feature Description
Synchronization of the presence of AdsLinks for a GA4 property was added in #8382 for the purpose of better informing when Consent Mode should be recommended. It currently queries the state once per day, however, due to the only other condition being GA4 connected it has become the most requested endpoint of the Admin API. Since this state is not highly time-sensitive, or likely to change often, we should greatly reduce the frequency in which it is called.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
includes/Modules/Analytics_4/Synchronize_AdsLinked.php
Context
instance to the list of arguments in the constructor and assign it to the$context
class propertymaybe_schedule_synchronize_ads_linked
methodis_admin() && 'googlesitekit-dashboard' === $this->context->input()->filter( INPUT_GET, 'page' )
)WEEK_IN_SECONDS
instead of a daysynchronize_ads_linked_data
method:Ads
module is connected to return early. You can usegooglesitekit_is_module_connected
filter for the checkincludes/Modules/Analytics_4.php
$context
class property to theSynchronize_AdsLinked
instance inregister
methodTest Coverage
QA Brief
Install
Advanced Cron Manager
plugin.Make sure
Ads
module is deactivated andAnalytics 4
module is connected.Go to
Tools > Cron Manager
, search forgooglesitekit_cron_synchronize_ads_linked_data
. If it is present, delete it.Go to Site Kit dashboard, let it load completely.
Go to
Tools > Cron Manager
again and search forgooglesitekit_cron_synchronize_ads_linked_data
. It should be available with the next execution set to 7 days (a week).Changelog entry