Skip to content
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

Added notification emails and basic scheduling logic #648

Draft
wants to merge 48 commits into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
33f9f7e
Initial work.
peterfabian Aug 5, 2024
6de19d2
Linting fix.
peterfabian Aug 5, 2024
3a78c28
Small improvements after rereading.
peterfabian Aug 5, 2024
7078d87
Consistent naming of actions.
peterfabian Aug 5, 2024
2aaf7e5
Consistent naming of classes and files.
peterfabian Aug 5, 2024
9c5284d
Added email triggers and simple check on action execute.
peterfabian Aug 5, 2024
8f8cda8
Handle reactivation correctly.
peterfabian Aug 6, 2024
ab9085e
Added order notes and actions.
peterfabian Aug 7, 2024
b614a4e
Add get/setter for hours offset and a filter.
peterfabian Aug 7, 2024
ab517ff
Made the change to autoloader less intrusive.
peterfabian Aug 7, 2024
41ea0c7
Removed todo after discussion.
peterfabian Aug 7, 2024
5a2dc00
Make behavior of enable/disable notifications consistent with other S…
peterfabian Aug 7, 2024
ade0f50
Added settings UI.
peterfabian Aug 7, 2024
e43558e
Merge branch 'trunk' into try/subs-notifications-pf
peterfabian Aug 8, 2024
707a4d2
Made the setting work.
peterfabian Aug 8, 2024
6afaabe
Merge branch 'try/subs-notifications-pf' of https://github.com/Automa…
peterfabian Aug 8, 2024
ffffa8f
Display only relevant actions for manual trigger.
peterfabian Aug 8, 2024
2dc1630
Allow the offset to be set based on subscription properties.
peterfabian Aug 8, 2024
31d38b6
Update notifications logic change.
peterfabian Sep 4, 2024
40750e8
Merge branch 'trunk' into try/subs-notifications-pf
peterfabian Sep 4, 2024
12dd88e
Added docs for new methods.
peterfabian Sep 4, 2024
5689328
Short array syntax.
peterfabian Sep 4, 2024
a6aef48
Don't unschedule expiry notification when subscription is cancelled.
peterfabian Sep 4, 2024
6a6e3ff
Fixing docs.
peterfabian Sep 4, 2024
a36b661
Change time offset to seconds.
peterfabian Sep 4, 2024
a2f613c
We still need those extra scheduling triggers.
peterfabian Sep 4, 2024
061ded4
Small refactor.
peterfabian Sep 4, 2024
da96aa8
Handle the case when subscription has neither modified, nor create date.
peterfabian Sep 4, 2024
1e2d51b
Improved docs.
peterfabian Sep 4, 2024
c240ace
Docs updated.
peterfabian Sep 4, 2024
7f1eca1
One renewal notification action should be enough for everyone.
peterfabian Sep 6, 2024
2bd7638
Use the getter instead of direct access.
peterfabian Sep 10, 2024
bc80cb1
Added checkbox to enable/disable notifications.
peterfabian Sep 10, 2024
fe361e6
Don't send notifications if globally disabled.
peterfabian Sep 10, 2024
43505cf
No notifications if subscription period is too short..
peterfabian Sep 10, 2024
3a32f2a
Added AS group name to notifications.
peterfabian Sep 11, 2024
8df46fd
Add batch processing to subscription notifications (#654)
peterfabian Sep 11, 2024
213901f
Merge branch 'trunk' into try/subs-notifications-pf
peterfabian Sep 11, 2024
094ce09
Don't schedule notifications if the killswitch is off.
peterfabian Sep 11, 2024
e467775
enable feature in tests
xristos3490 Sep 11, 2024
51ca698
Move subscription_period_too_short as the email class often isn't loa…
peterfabian Sep 11, 2024
2e7ce00
Fix unit
xristos3490 Sep 11, 2024
aa03ca3
Fix units (2)
xristos3490 Sep 11, 2024
39b61e1
Make get_action_args static, as it doesn't depend on instance.
peterfabian Sep 13, 2024
e01046b
Handle conflict between trial end and next payment notification.
peterfabian Sep 17, 2024
97a9198
Improved settting wording.
peterfabian Sep 17, 2024
956d3f3
Made settings consistent.
peterfabian Sep 17, 2024
f6bb953
Merge branch 'trunk' into try/subs-notifications-pf
peterfabian Sep 18, 2024
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
19 changes: 19 additions & 0 deletions assets/js/admin/admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,9 @@ jQuery( function ( $ ) {
),
$syncRenewals = $(
document.getElementById( 'woocommerce_subscriptions_sync_payments' )
),
$customerNotifications = $(
document.getElementById( 'woocommerce_subscriptions_customer_notifications_enabled' )
);

// We're on the Subscriptions settings page
Expand All @@ -954,6 +957,9 @@ jQuery( function ( $ ) {
).parents( 'tr' ),
$suspensionExtensionRow = $(
'#woocommerce_subscriptions_recoup_suspension'
).parents( 'tr' ),
$customerNotificationOffsetRow = $(
'#woocommerce_subscriptions_customer_notifications_offset'
).parents( 'tr' );

// No animation for initial hiding when switching is disabled.
Expand Down Expand Up @@ -1009,6 +1015,19 @@ jQuery( function ( $ ) {
$daysNoFeeRow.fadeOut();
}
} );

// No animation when initially hiding customer notification offset row.
if ( ! $customerNotifications.is( ':checked' ) ) {
$customerNotificationOffsetRow.hide();
}
// Watch the enable/disable customer notifications checkbox for changes.
$customerNotifications.on( 'change', function () {
if ( $( this ).is( ':checked' ) ) {
$customerNotificationOffsetRow.fadeIn();
} else {
$customerNotificationOffsetRow.fadeOut();
}
} );
}

// Don't display the variation notice for variable subscription products
Expand Down
18 changes: 18 additions & 0 deletions bin/phpcs-2.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/usr/bin/env bash

ROOTDIR="$(dirname "$(dirname "$0")")"
echo $ROOTDIR

# Run PHP CodeSniffer and capture the output
PHPCS_OUTPUT=$($ROOTDIR/vendor/bin/phpcs --report=json "$@")

# Check if the output is valid JSON
if echo "$PHPCS_OUTPUT" | jq empty >/dev/null 2>&1; then
# If valid JSON, pass it to sarb
echo "$PHPCS_OUTPUT" | $ROOTDIR/vendor/bin/sarb remove phpcs.baseline
else
# If not valid JSON, print an error message and the invalid output
echo "Failed to parse analysis results. Not valid JSON."
echo "$PHPCS_OUTPUT"
exit 1
fi
Original file line number Diff line number Diff line change
@@ -0,0 +1,311 @@
<?php

use Automattic\WooCommerce\Internal\BatchProcessing\BatchProcessingController;
use Automattic\WooCommerce\Internal\BatchProcessing\BatchProcessorInterface;

class WCS_Notifications_Debug_Tool_Processor implements BatchProcessorInterface {

/**
* Option name for the tool state.
* This is used to pass the state of the tool between requests.
*/
const TOOL_STATE_OPTION_NAME = 'wcs_notifications_debug_tool_state';

/**
* Constructor.
*/
public function __construct() {
add_filter( 'woocommerce_debug_tools', array( $this, 'handle_woocommerce_debug_tools' ), 999, 1 );
}

/**
* Get the state of the tool.
*
* @return array {
* @last_offset Last offset processed.
* }
*/
private function get_tool_state(): array {
return (array) get_option( self::TOOL_STATE_OPTION_NAME, array() );
}

/**
* Update the state of the tool.
*
* @param array $state New state of the tool.
*/
private function update_tool_state( $state ) {
update_option( self::TOOL_STATE_OPTION_NAME, $state );
}

/**
* Delete the state of the tool.
*/
private function delete_tool_state() {
delete_option( self::TOOL_STATE_OPTION_NAME );
}

/**
* Get a user-friendly name for this processor.
*
* @return string Name of the processor.
*/
public function get_name(): string {
return 'wcs_notifications_debug_tool_processor';
}

/**
* Get a user-friendly description for this processor.
*
* @return string Description of what this processor does.
*/
public function get_description(): string {
return 'WooCommerce Notifications Debug Tool Processor';
}

/**
* Get the allowed subscription statuses to process.
*/
protected function get_subscription_statuses(): array {
$allowed_statuses = array(
'active',
'pending',
'on-hold',
);

return array_map( 'wcs_sanitize_subscription_status_key', $allowed_statuses );
}

public function get_total_pending_count(): int {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: This function is missing a function comment.

global $wpdb;

$allowed_statuses = $this->get_subscription_statuses();
$placeholders = implode( ', ', array_fill( 0, count( $allowed_statuses ), '%s' ) );

if ( wcs_is_custom_order_tables_usage_enabled() ) {
$total_subscriptions = $wpdb->get_var(
// phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared
// phpcs:disable WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber
// phpcs:disable WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare
$wpdb->prepare(
"SELECT
COUNT(id)
FROM {$wpdb->prefix}wc_orders
WHERE type='shop_subscription'
AND status IN ($placeholders)
",
...$allowed_statuses
)
);
} else {
$total_subscriptions = $wpdb->get_var(
// phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared
// phpcs:disable WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber
// phpcs:disable WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare
$wpdb->prepare(
"SELECT
COUNT(ID)
FROM {$wpdb->prefix}posts
WHERE post_type='shop_subscription'
AND post_status IN ($placeholders)
",
...$allowed_statuses
)
);
}

$state = $this->get_tool_state();
if ( isset( $state['last_offset'] ) ) {
$total_subscriptions -= (int) $state['last_offset'];
}

return $total_subscriptions;
}

/**
* Returns the next batch of items that need to be processed.
*
* A batch item can be anything needed to identify the actual processing to be done,
* but whenever possible items should be numbers (e.g. database record ids)
* or at least strings, to ease troubleshooting and logging in case of problems.
*
* The size of the batch returned can be less than $size if there aren't that
* many items pending processing (and it can be zero if there isn't anything to process),
* but the size should always be consistent with what 'get_total_pending_count' returns
* (i.e. the size of the returned batch shouldn't be larger than the pending items count).
*
* @param int $size Maximum size of the batch to be returned.
*
* @return array Batch of items to process, containing $size or less items.
*/
public function get_next_batch_to_process( int $size ): array {
global $wpdb;

$allowed_statuses = $this->get_subscription_statuses();
$placeholders = implode( ', ', array_fill( 0, count( $allowed_statuses ), '%s' ) );
$state = $this->get_tool_state();
$offset = isset( $state['last_offset'] ) ? (int) $state['last_offset'] : 0;

$args = array_merge(
$allowed_statuses,
array( $size ),
array( $offset ),
);

if ( wcs_is_custom_order_tables_usage_enabled() ) {
$subscriptions_to_process = $wpdb->get_col(
// phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared
// phpcs:disable WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber
$wpdb->prepare(
"SELECT
id
FROM {$wpdb->prefix}wc_orders
WHERE type='shop_subscription'
AND status IN ($placeholders)
ORDER BY id ASC
LIMIT %d
OFFSET %d",
...$args
)
);
} else {
$subscriptions_to_process = $wpdb->get_col(
// phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared
// phpcs:disable WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber
$wpdb->prepare(
"SELECT
ID
FROM {$wpdb->prefix}posts
WHERE post_type='shop_subscription'
AND post_status IN ($placeholders)
ORDER BY ID ASC
LIMIT %d
OFFSET %d",
...$args
)
);
}

// Reset the tool state if there are no more subscriptions to process.
if ( empty( $subscriptions_to_process ) ) {
$this->delete_tool_state();
}

return $subscriptions_to_process;
}

/**
* Process data for the supplied batch.
*
* This method should be prepared to receive items that don't actually need processing
* (because they have been processed before) and ignore them, but if at least
* one of the batch items that actually need processing can't be processed, an exception should be thrown.
*
* Once an item has been processed it shouldn't be counted in 'get_total_pending_count'
* nor included in 'get_next_batch_to_process' anymore (unless something happens that causes it
* to actually require further processing).
*
* @throw \Exception Something went wrong while processing the batch.
*
* @param array $batch Batch to process, as returned by 'get_next_batch_to_process'.
*/
public function process_batch( array $batch ): void {
// This is a bit unnecessary. Perhaps convert `update_status` to static to avoid instantiating the class?
$subscriptions_notifications = new WCS_Action_Scheduler_Customer_Notifications();

foreach ( $batch as $subscription_id ) {
$subscription = wcs_get_subscription( $subscription_id );
$subscriptions_notifications->update_status( $subscription, $subscription->get_status(), null );

// Update the subscription's update time to mark it as updated.
$subscription->set_date_modified( time() );
$subscription->save();
}

// Update tool state.
$state = $this->get_tool_state();
$state['last_offset'] = isset( $state['last_offset'] ) ? absint( $state['last_offset'] ) + count( $batch ) : count( $batch );
$this->update_tool_state( $state );
}

/**
* Default (preferred) batch size to pass to 'get_next_batch_to_process'.
* The controller will pass this size unless it's externally configured
* to use a different size.
*
* @return int Default batch size.
*/
public function get_default_batch_size(): int {
return 20;
}

/**
* Start the background process for coupon data conversion.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Start the background process for coupon data conversion.
* Start the background process for batch processing subscription notifications updates.

Looks like a copy-paste.

*
* @return string Informative string to show after the tool is triggered in UI.
*/
public function enqueue(): string {
$batch_processor = wc_get_container()->get( BatchProcessingController::class );
if ( $batch_processor->is_enqueued( self::class ) ) {
return __( 'Background process for updating subscription notifications already started, nothing done.', 'woocommerce-subscriptions' );
}

$batch_processor->enqueue_processor( self::class );
return __( 'Background process for updating subscription notifications started', 'woocommerce-subscriptions' );
}

/**
* Stop the background process for coupon data conversion.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Stop the background process for coupon data conversion.
* Stop the background process for batch processing subscription notifications updates.

*
* @return string Informative string to show after the tool is triggered in UI.
*/
public function dequeue(): string {
$batch_processor = wc_get_container()->get( BatchProcessingController::class );
if ( ! $batch_processor->is_enqueued( self::class ) ) {
return __( 'Background process for updating subscription notifications not started, nothing done.', 'woocommerce-subscriptions' );
}

$batch_processor->remove_processor( self::class );
return __( 'Background process for updating subscription notifications stopped', 'woocommerce-subscriptions' );
}

/**
* Add the tool to start or stop the background process that manages notification batch processing.
*
* @param array $tools Old tools array.
* @return array Updated tools array.
*/
public function handle_woocommerce_debug_tools( array $tools ): array {
$batch_processor = wc_get_container()->get( BatchProcessingController::class );
$pending_count = $this->get_total_pending_count();

if ( 0 === $pending_count ) {
$tools['start_add_subscription_notifications'] = array(
'name' => __( 'Start adding subscription notifications', 'woocommerce-subscriptions' ),
'button' => __( 'Add notifications', 'woocommerce-subscriptions' ),
'disabled' => true,
'desc' => __( 'This tool will add notifications to pending, active and on-hold subscriptions. This will happen overtime in the background (via Action Scheduler). There are currently no entries to convert.', 'woocommerce-subscriptions' ),
);
} elseif ( $batch_processor->is_enqueued( self::class ) ) {
$tools['stop_add_subscription_notifications'] = array(
'name' => __( 'Stop adding subscription notifications', 'woocommerce-subscriptions' ),
'button' => __( 'Stop adding notifications', 'woocommerce-subscriptions' ),
'desc' =>
/* translators: %d=count of entries pending conversion */
sprintf( __( 'This will stop the background process that adds notifications to pending, active and on-hold subscriptions. There are currently %d entries that can be converted.', 'woocommerce-subscriptions' ), $pending_count ),
'callback' => array( $this, 'dequeue' ),
);
} else {
$tools['start_add_subscription_notifications'] = array(
'name' => __( 'Start adding subscription notifications', 'woocommerce-subscriptions' ),
'button' => __( 'Add notifications', 'woocommerce-subscriptions' ),
'desc' =>
/* translators: %d=count of entries pending conversion */
sprintf( __( 'This tool will add notifications to pending, active and on-hold subscriptions. This will happen overtime in the background (via Action Scheduler). There are currently %d entries that can be converted.', 'woocommerce-subscriptions' ), $pending_count ),
'callback' => array( $this, 'enqueue' ),
);
}

return $tools;
}
}
Loading
Loading