Skip to content

Bug/12385 monthly frequency #12414

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.

Sign up for GitHub

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

Open
zutigrm wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Bug/12385 monthly frequency #12414

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7e5766f
Change monthly period label.
zutigrm Mar 31, 2026
b6068dd
Compute monthly window from previous month length.
zutigrm Mar 31, 2026
70eb76a
Cover monthly 28/29/30/31 day windows.
zutigrm Mar 31, 2026
fe1ff61
Update snapshot.
zutigrm Mar 31, 2026
3f5f57f
Update test which caused warninigs.
zutigrm Mar 31, 2026
9745bdf
Update VRT.
zutigrm Mar 31, 2026
c893652
Calculate quarterly period dynamically.
zutigrm Apr 2, 2026
e6cf40a
Rename sendDate to endDate.
zutigrm Apr 2, 2026
ddd1443
Merge remote-tracking branch 'origin/develop' into bug/12385-monthly-…
zutigrm Apr 2, 2026
042cdfc
Rename to .
zutigrm Apr 2, 2026
dfd4276
Update VRT.
zutigrm Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion assets/js/components/email-reporting/FrequencySelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export default function FrequencySelector( { isUserSubscribed, isLoading } ) {
},
monthly: {
label: __( 'Monthly', 'google-site-kit' ),
period: __( 'Last 28 days', 'google-site-kit' ),
period: __( 'Last month', 'google-site-kit' ),
description: __(
'Sent on the 1st of each month',
'google-site-kit'
Expand Down
7 changes: 7 additions & 0 deletions assets/js/components/email-reporting/UserSettingsSelectionPanel/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ import {
import UserSettingsSelectionPanel from '@/js/components/email-reporting/UserSettingsSelectionPanel';
import SelectionPanelFooter from '@/js/components/email-reporting/UserSettingsSelectionPanel/SelectionPanelFooter';

// This suite tests panel behavior; mock the invite list to avoid async datastore
// updates from child-level fetching that are covered in InviteOthersToSubscribe tests.
jest.mock(
'@/js/components/email-reporting/InviteOthersToSubscribe',
() => () => null
);

describe( 'UserSettingsSelectionPanel', () => {
const features = [ 'proactiveUserEngagement' ];
const emailReportingSettingsEndpoint = new RegExp(
Expand Down
12 changes: 6 additions & 6 deletions assets/js/components/email-reporting/__snapshots__/FrequencySelector.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ exports[`FrequencySelector Story states (visual + DOM) Monthly selected renders
<span
class="googlesitekit-typography googlesitekit-typography--body googlesitekit-typography--small"
>
Last 28 days
Last month
</span>
</div>
<div
Expand Down Expand Up @@ -210,7 +210,7 @@ exports[`FrequencySelector Story states (visual + DOM) Previously saved frequenc
<span
class="googlesitekit-typography googlesitekit-typography--body googlesitekit-typography--small"
>
Last 28 days
Last month
</span>
</div>
<div
Expand Down Expand Up @@ -345,7 +345,7 @@ exports[`FrequencySelector Story states (visual + DOM) Previously saved frequenc
<span
class="googlesitekit-typography googlesitekit-typography--body googlesitekit-typography--small"
>
Last 28 days
Last month
</span>
</div>
<div
Expand Down Expand Up @@ -480,7 +480,7 @@ exports[`FrequencySelector Story states (visual + DOM) Quarterly selected render
<span
class="googlesitekit-typography googlesitekit-typography--body googlesitekit-typography--small"
>
Last 28 days
Last month
</span>
</div>
<div
Expand Down Expand Up @@ -615,7 +615,7 @@ exports[`FrequencySelector Story states (visual + DOM) Weekly selected (default
<span
class="googlesitekit-typography googlesitekit-typography--body googlesitekit-typography--small"
>
Last 28 days
Last month
</span>
</div>
<div
Expand Down Expand Up @@ -750,7 +750,7 @@ exports[`FrequencySelector Story states (visual + DOM) Weekly selected with Sund
<span
class="googlesitekit-typography googlesitekit-typography--body googlesitekit-typography--small"
>
Last 28 days
Last month
</span>
</div>
<div
Expand Down
5 changes: 4 additions & 1 deletion includes/Core/Email_Reporting/Initiator_Task.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,15 @@ public static function build_reference_dates( $frequency, $timestamp ) {

$period_lengths = array(
Email_Reporting_Settings::FREQUENCY_WEEKLY => 7,
Email_Reporting_Settings::FREQUENCY_MONTHLY => 30,
Email_Reporting_Settings::FREQUENCY_QUARTERLY => 90,
Copy link
Copy Markdown
Collaborator

@aaemnnosttv aaemnnosttv Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't use a fixed number of days here as the only period which has a fixed length is weekly.

Copy link
Copy Markdown
Collaborator

@tofumatt tofumatt Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, as part of this issue (even though it's not mentioned in the ACs), we're best off using the number of days in the respective quarter, same as we do for each month.

Originally I thought that this constant is inaccurately-named, as the text is actually "last 90 days", but we should either label the intent is clearly to send an email every quarter:

Image

Let's update this as part of this issue as well.

);

$period_days = isset( $period_lengths[ $frequency ] ) ? $period_lengths[ $frequency ] : $period_lengths[ Email_Reporting_Settings::FREQUENCY_WEEKLY ];

if ( Email_Reporting_Settings::FREQUENCY_MONTHLY === $frequency ) {
$period_days = (int) $send_date->format( 't' );
}

// endDate is inclusive, so startDate must be endDate - (period_days - 1) for an exact period-length window.
$start_date = $send_date->sub( new DateInterval( sprintf( 'P%dD', max( $period_days - 1, 0 ) ) ) );
$compare_end_date = $start_date->sub( new DateInterval( 'P1D' ) );
Expand Down
49 changes: 43 additions & 6 deletions tests/phpunit/integration/Core/Email_Reporting/Initiator_TaskTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,14 @@ public function test_handle_callback_action_without_subscribers_still_schedules_
/**
* @dataProvider data_build_reference_dates_uses_expected_period_length
*/
public function test_build_reference_dates_uses_expected_period_length( $frequency, $expected_days ) {
public function test_build_reference_dates_uses_expected_period_length( $frequency, $expected_days, $timestamp ) {
$original_timezone_string = get_option( 'timezone_string' );
$original_gmt_offset = get_option( 'gmt_offset' );

update_option( 'timezone_string', 'UTC' );
update_option( 'gmt_offset', 0 );

try {
$timestamp = strtotime( '2026-03-16 00:00:00 UTC' );

$reference_dates = Initiator_Task::build_reference_dates(
$frequency,
$timestamp
Expand All @@ -232,6 +230,36 @@ public function test_build_reference_dates_uses_expected_period_length( $frequen
}
}

/**
* @dataProvider data_build_reference_dates_monthly_uses_previous_month_window
*/
public function test_build_reference_dates_monthly_uses_previous_month_window( $scheduled_timestamp, $expected_start, $expected_send, $expected_days ) {
$original_timezone_string = get_option( 'timezone_string' );
$original_gmt_offset = get_option( 'gmt_offset' );

update_option( 'timezone_string', 'UTC' );
update_option( 'gmt_offset', 0 );

try {
$reference_dates = Initiator_Task::build_reference_dates(
Email_Reporting_Settings::FREQUENCY_MONTHLY,
$scheduled_timestamp
);

$this->assertSame( $expected_start, $reference_dates['startDate'], 'Expected monthly startDate to be first day of previous month.' );
$this->assertSame( $expected_send, $reference_dates['sendDate'], 'Expected monthly sendDate to be last day of previous month.' );

$current_start = new \DateTimeImmutable( $reference_dates['startDate'] );
$current_end = new \DateTimeImmutable( $reference_dates['sendDate'] );
$current_days = (int) $current_start->diff( $current_end )->days + 1;

$this->assertSame( $expected_days, $current_days, 'Expected monthly range length to match previous month day count.' );
} finally {
update_option( 'timezone_string', $original_timezone_string );
update_option( 'gmt_offset', $original_gmt_offset );
}
}

public function test_build_reference_dates_uses_previous_day_as_send_date() {
$original_timezone_string = get_option( 'timezone_string' );
$original_gmt_offset = get_option( 'gmt_offset' );
Expand All @@ -256,9 +284,18 @@ public function test_build_reference_dates_uses_previous_day_as_send_date() {

public function data_build_reference_dates_uses_expected_period_length() {
return array(
'weekly' => array( Email_Reporting_Settings::FREQUENCY_WEEKLY, 7 ),
'monthly' => array( Email_Reporting_Settings::FREQUENCY_MONTHLY, 30 ),
'quarterly' => array( Email_Reporting_Settings::FREQUENCY_QUARTERLY, 90 ),
'weekly' => array( Email_Reporting_Settings::FREQUENCY_WEEKLY, 7, strtotime( '2026-03-16 00:00:00 UTC' ) ),
'monthly' => array( Email_Reporting_Settings::FREQUENCY_MONTHLY, 28, strtotime( '2026-03-01 00:00:00 UTC' ) ),
'quarterly' => array( Email_Reporting_Settings::FREQUENCY_QUARTERLY, 90, strtotime( '2026-03-16 00:00:00 UTC' ) ),
);
}

public function data_build_reference_dates_monthly_uses_previous_month_window() {
return array(
'previous month has 28 days' => array( strtotime( '2026-03-01 00:00:00 UTC' ), '2026-02-01', '2026-02-28', 28 ),
'previous month has 29 days (leap year)' => array( strtotime( '2024-03-01 00:00:00 UTC' ), '2024-02-01', '2024-02-29', 29 ),
'previous month has 30 days' => array( strtotime( '2026-05-01 00:00:00 UTC' ), '2026-04-01', '2026-04-30', 30 ),
'previous month has 31 days' => array( strtotime( '2026-08-01 00:00:00 UTC' ), '2026-07-01', '2026-07-31', 31 ),
);
}
}
Loading