Repository navigation
hotfix/cp-12676-cleverpush-wordpress-plugin-not-saving-scheduled-push - #15
paavan-cleverpush wants to merge 1 commit into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f288555. Configure here.
| $saved_timestamp = !empty($saved_scheduled_at) ? strtotime($saved_scheduled_at) : false; | ||
| if ($saved_timestamp && $saved_timestamp > time()) { | ||
| $options['scheduledAt'] = $saved_scheduled_at; | ||
| } |
There was a problem hiding this comment.
Stale schedule used on publish
Medium Severity
The new scheduledAt fallback always reads saved post meta when $_POST['cleverpush_scheduled_at'] is empty, even if the metabox was submitted with a cleared date. publish_post runs before save_post, so a previously stored time can still be attached and the push is delayed instead of sending immediately.
Reviewed by Cursor Bugbot for commit f288555. Configure here.
| $options['tags'] = array_map('sanitize_text_field', $_POST['cleverpush_tags']); | ||
| } | ||
| $thumbnail_url = get_the_post_thumbnail_url(); | ||
| $thumbnail_url = get_the_post_thumbnail_url($post_id); |
There was a problem hiding this comment.
Targeting lost on scheduled publish
Medium Severity
The new cron publish path sends without segments, topics, or tags. Those targeting fields are only read from $_POST and are never persisted, so a scheduled WordPress post is delivered to all subscribers instead of the selected audience.
Reviewed by Cursor Bugbot for commit f288555. Configure here.
|
|
||
| if (!isset($_POST['cleverpush_send_notification'])) { | ||
| $this->delete_draft_notification($post_id); | ||
| return; |
There was a problem hiding this comment.
Drafts linger after post removal
Low Severity
sync_draft_notification creates a CleverPush draft but only removes it when the send checkbox is cleared or the post later publishes. Trashing, deleting, or otherwise leaving draft, future, or pending leaves that remote draft in place.
Reviewed by Cursor Bugbot for commit f288555. Configure here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cleverpush-api.php">
<violation number="1" location="cleverpush-api.php:97">
P2: `delete_notification()` returns transport and API failures without giving the caller a success/failure contract. `delete_draft_notification()` then removes both local identifiers unconditionally, so a failed DELETE leaves the remote draft orphaned and prevents later retry; propagate the deletion result and clear metadata only after a successful response.</violation>
</file>
<file name="cleverpush.php">
<violation number="1" location="cleverpush.php:1153">
P1: Scheduled cron publishes now send to all subscriptions instead of the segments, topics, or tags selected in the metabox. Persist those selections and load them from post metadata when `$_POST` is unavailable.</violation>
<violation number="2" location="cleverpush.php:1305">
P3: Delete the remote draft when the post leaves these statuses and when the post is deleted; this early return skips cleanup, leaving the CleverPush draft behind.</violation>
<violation number="3" location="cleverpush.php:1314">
P2: Draft synchronization ignores the `cleverpush_notification_title_required` setting and creates a draft from the post title. Apply the same required-headline rule before sending the draft.</violation>
<violation number="4" location="cleverpush.php:1322">
P2: Clearing a draft’s title leaves its previously synchronized CleverPush draft orphaned because this early return skips cleanup. Delete the tracked draft before returning when no valid title remains.</violation>
<violation number="5" location="cleverpush.php:1332">
P2: The draft deduplication hash omits the notification URL, so permalink changes leave the remote draft linking to the previous post URL. Include the current permalink in the fingerprint.</violation>
<violation number="6" location="cleverpush.php:1339">
P2: Changing a draft deletes the existing CleverPush draft before creating its replacement. If the replacement request fails, the old draft is already gone; create the replacement first and delete the old notification only after the new ID and hash are stored.</violation>
</file>
Tip: Get a Slack DM when cubic finishes reviewing your PRs. Connect Slack
View guided diff | Turn on auto-fix | Re-trigger cubic
| if (!current_user_can('cleverpush_send')) { | ||
| $saved_send = get_post_meta($post_id, 'cleverpush_send_notification', true); | ||
| $publishing_via_cron = (function_exists('wp_doing_cron') && wp_doing_cron()) || (defined('DOING_CRON') && DOING_CRON); | ||
| if (!current_user_can('cleverpush_send') && !($publishing_via_cron && $saved_send)) { |
There was a problem hiding this comment.
P1: Scheduled cron publishes now send to all subscriptions instead of the segments, topics, or tags selected in the metabox. Persist those selections and load them from post metadata when $_POST is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cleverpush.php, line 1153:
<comment>Scheduled cron publishes now send to all subscriptions instead of the segments, topics, or tags selected in the metabox. Persist those selections and load them from post metadata when `$_POST` is unavailable.</comment>
<file context>
@@ -1126,16 +1148,18 @@ public function publish_post($post_id)
- if (!current_user_can('cleverpush_send')) {
+ $saved_send = get_post_meta($post_id, 'cleverpush_send_notification', true);
+ $publishing_via_cron = (function_exists('wp_doing_cron') && wp_doing_cron()) || (defined('DOING_CRON') && DOING_CRON);
+ if (!current_user_can('cleverpush_send') && !($publishing_via_cron && $saved_send)) {
return;
}
</file context>
| return null; | ||
| } | ||
|
|
||
| return wp_remote_request( |
There was a problem hiding this comment.
P2: delete_notification() returns transport and API failures without giving the caller a success/failure contract. delete_draft_notification() then removes both local identifiers unconditionally, so a failed DELETE leaves the remote draft orphaned and prevents later retry; propagate the deletion result and clear metadata only after a successful response.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cleverpush-api.php, line 97:
<comment>`delete_notification()` returns transport and API failures without giving the caller a success/failure contract. `delete_draft_notification()` then removes both local identifiers unconditionally, so a failed DELETE leaves the remote draft orphaned and prevents later retry; propagate the deletion result and clear metadata only after a successful response.</comment>
<file context>
@@ -85,6 +85,27 @@ public static function send_notification($title, $body, $url, $options = array()
+ return null;
+ }
+
+ return wp_remote_request(
+ CLEVERPUSH_API_ENDPOINT . '/channel/' . rawurlencode($channel_id) . '/notification/' . rawurlencode($notification_id),
+ array(
</file context>
| $scheduled_at = ''; | ||
| } | ||
|
|
||
| $fingerprint = md5($title . '|' . $text . '|' . $scheduled_at); |
There was a problem hiding this comment.
P2: The draft deduplication hash omits the notification URL, so permalink changes leave the remote draft linking to the previous post URL. Include the current permalink in the fingerprint.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cleverpush.php, line 1332:
<comment>The draft deduplication hash omits the notification URL, so permalink changes leave the remote draft linking to the previous post URL. Include the current permalink in the fingerprint.</comment>
<file context>
@@ -1245,6 +1291,86 @@ public function save_post($post_id, $post)
+ $scheduled_at = '';
+ }
+
+ $fingerprint = md5($title . '|' . $text . '|' . $scheduled_at);
+ $existing_id = get_post_meta($post_id, 'cleverpush_draft_notification_id', true);
+ $existing_fingerprint = get_post_meta($post_id, 'cleverpush_draft_notification_hash', true);
</file context>
| $fingerprint = md5($title . '|' . $text . '|' . $scheduled_at); | |
| $fingerprint = md5($title . '|' . $text . '|' . $scheduled_at . '|' . get_permalink($post_id)); |
| if (empty($title)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
P2: Clearing a draft’s title leaves its previously synchronized CleverPush draft orphaned because this early return skips cleanup. Delete the tracked draft before returning when no valid title remains.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cleverpush.php, line 1322:
<comment>Clearing a draft’s title leaves its previously synchronized CleverPush draft orphaned because this early return skips cleanup. Delete the tracked draft before returning when no valid title remains.</comment>
<file context>
@@ -1245,6 +1291,86 @@ public function save_post($post_id, $post)
+ if (!empty($_POST['cleverpush_text'])) {
+ $text = sanitize_text_field(wp_unslash($_POST['cleverpush_text']));
+ }
+ if (empty($title)) {
+ return;
+ }
</file context>
| if (empty($title)) { | |
| return; | |
| } | |
| if (empty($title)) { | |
| $this->delete_draft_notification($post_id); | |
| return; | |
| } |
| return; | ||
| } | ||
|
|
||
| $title = html_entity_decode(get_the_title($post_id)); |
There was a problem hiding this comment.
P2: Draft synchronization ignores the cleverpush_notification_title_required setting and creates a draft from the post title. Apply the same required-headline rule before sending the draft.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cleverpush.php, line 1314:
<comment>Draft synchronization ignores the `cleverpush_notification_title_required` setting and creates a draft from the post title. Apply the same required-headline rule before sending the draft.</comment>
<file context>
@@ -1245,6 +1291,86 @@ public function save_post($post_id, $post)
+ return;
+ }
+
+ $title = html_entity_decode(get_the_title($post_id));
+ if (!empty($_POST['cleverpush_title'])) {
+ $title = sanitize_text_field(wp_unslash($_POST['cleverpush_title']));
</file context>
| $title = html_entity_decode(get_the_title($post_id)); | |
| $title = get_option('cleverpush_notification_title_required') == 'on' ? '' : html_entity_decode(get_the_title($post_id)); |
| return; | ||
| } | ||
|
|
||
| if (!empty($existing_id)) { |
There was a problem hiding this comment.
P2: Changing a draft deletes the existing CleverPush draft before creating its replacement. If the replacement request fails, the old draft is already gone; create the replacement first and delete the old notification only after the new ID and hash are stored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cleverpush.php, line 1339:
<comment>Changing a draft deletes the existing CleverPush draft before creating its replacement. If the replacement request fails, the old draft is already gone; create the replacement first and delete the old notification only after the new ID and hash are stored.</comment>
<file context>
@@ -1245,6 +1291,86 @@ public function save_post($post_id, $post)
+ return;
+ }
+
+ if (!empty($existing_id)) {
+ $this->delete_draft_notification($post_id);
+ }
</file context>
| } | ||
|
|
||
| $status = get_post_status($post_id); | ||
| if (!in_array($status, array('draft', 'future', 'pending'), true)) { |
There was a problem hiding this comment.
P3: Delete the remote draft when the post leaves these statuses and when the post is deleted; this early return skips cleanup, leaving the CleverPush draft behind.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cleverpush.php, line 1305:
<comment>Delete the remote draft when the post leaves these statuses and when the post is deleted; this early return skips cleanup, leaving the CleverPush draft behind.</comment>
<file context>
@@ -1245,6 +1291,86 @@ public function save_post($post_id, $post)
+ }
+
+ $status = get_post_status($post_id);
+ if (!in_array($status, array('draft', 'future', 'pending'), true)) {
+ return;
+ }
</file context>
Note
Medium Risk
Changes core publish/save notification flows and adds remote API create/delete on every draft save; wrong meta or cron handling could miss or duplicate pushes, but scope stays within CleverPush notification delivery.
Overview
Fixes scheduled CleverPush notifications not surviving saves and not firing when WordPress publishes (including cron-driven scheduled posts).
Scheduling in the editor now keeps the hidden
cleverpush_scheduled_atin sync with the datetime picker on submit and Gutenberg save, clears both fields when push is disabled, and only writes schedule meta when the metabox was actually submitted—so saves no longer wipe a stored schedule.Publish path reads saved meta for send flag, custom title/text, and future
scheduledAtwhen the metabox is not in the request (typical on scheduled publish), and allowspublish_postto run under cron when a send was previously saved.CleverPush draft sync creates or updates remote draft notifications for unpublished posts (with hash-based dedup), removes them when push is unchecked or after a successful send, via new
delete_notificationAPI support. Plugin version 1.9.9.Reviewed by Cursor Bugbot for commit f288555. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the scheduled push notification flow so schedules persist and fire on publish without losing draft state.
Written for commit f288555. Summary will update on new commits.