Unschedule send usage event if it is disabled#2143
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
classes/controllers/FrmUsageController.php (3)
29-36: LGTM! Consider adding debug logging.The unscheduling logic is well-implemented. For better debugging capabilities, consider adding a debug log when events are unscheduled.
if ( $timestamp ) { + if ( defined('WP_DEBUG') && WP_DEBUG ) { + error_log('Formidable: Unscheduling usage tracking event due to disabled tracking'); + } // Remove the scheduled event if it's not allowed and it's scheduled. wp_unschedule_event( $timestamp, 'formidable_send_usage' ); }
29-36: Consider adding a transient to prevent rapid re-scheduling attempts.To optimize performance and prevent unnecessary checks, consider adding a transient that tracks the scheduling state.
$timestamp = wp_next_scheduled( 'formidable_send_usage' ); +$cache_key = 'frm_usage_schedule_check'; +if ( get_transient( $cache_key ) ) { + return; +} +set_transient( $cache_key, true, HOUR_IN_SECONDS ); + if ( ! self::tracking_allowed() ) {
71-71: Update the version tag with the actual version number.The
@since x.xtag should be replaced with the actual version number where this method was introduced.- * @since x.x + * @since 6.16.2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
classes/controllers/FrmUsageController.php(2 hunks)classes/models/FrmUsage.php(1 hunks)
🔇 Additional comments (2)
classes/controllers/FrmUsageController.php (1)
29-36: Verify the implementation with WP Crontrol plugin.
The implementation looks correct, but please verify:
- Event is unscheduled when tracking is disabled
- Event remains scheduled when tracking is enabled
classes/models/FrmUsage.php (1)
451-451: LGTM! Good refactoring to centralize tracking permission logic.
The change follows good software design principles by delegating the tracking permission check to the controller class.
Let's verify the complete implementation of the tracking permission feature:
✅ Verification successful
Verified: Event unscheduling is properly implemented when tracking is disabled
The verification confirms that:
- The
tracking_allowed()check is properly implemented inFrmUsageControllerand used consistently - Event unscheduling logic is correctly implemented in
FrmUsageController:- Checks if tracking is allowed
- Unschedules the
formidable_send_usageevent when tracking is disabled - Only schedules the event when tracking is allowed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of tracking permission and event scheduling
# Check FrmUsageController implementation
echo "Checking FrmUsageController implementation..."
ast-grep --pattern 'class FrmUsageController {
$$$
public static function tracking_allowed() {
$$$
}
$$$
}'
# Check event scheduling/unscheduling logic
echo "Checking event scheduling logic..."
rg -A 5 'formidable_send_usage'
# Check if there are any other files using the tracking permission check
echo "Checking usage of tracking permission check..."
rg 'tracking_allowed'
Length of output: 3336
Crabcyborg
left a comment
Crabcyborg
left a comment
There was a problem hiding this comment.
Thank you @truongwp!
🚀
2 participants
Fixes https://github.com/Strategy11/formidable-pro/issues/4291
To test, you can use the WP Crontrol to see the list of scheduled events, turn on and off the Send usage feature in the Miscellaneous tab in the Global settings and check if the
formidable_send_usageevent is added or removed.