search

OneSignal / OneSignal-WordPress-Plugin

OneSignal is a free push notification service for web and mobile apps. This plugin makes it easy to integrate your website with OneSignal Push Notifications. https://onesignal.com
Other
80 stars 42 forks source link

Prevent notifying updates for non-public post types #251

Closed westonruter closed 4 years ago

westonruter commented 4 years ago

A user reported an issue for the official AMP plugin where the OneSignal plugin was sending out push notifications for updates to the non-public amp_validated_url post type: https://github.com/ampproject/amp-wp/issues/5297

When the AMP plugin's Developer Tools are enabled, each save for a post will trigger a loopback request to check the AMP validity of the post on the frontend. The results are then stored in an amp_validated_url post type. While this post type is private, the posts are stored with the publish status. Nevertheless, they cannot be viewed on the frontend.

OneSignal is incorrectly detecting updates to the published amp_validated_url posts as needing push notifications. This likely is an issue for other post types in WordPress, including core's customize_changeset and nav_menu_item post types.

In looking at the logic in \OneSignal_Admin::send_notification_on_wp_post() I see no checks for whether the post type being saved is actually public, in particular when $posted_from_wordpress_editor is true.

This PR should offer a quick way to prevent this from happening.

This change is Reviewable

westonruter commented 4 years ago

This fixed the issue according to user testing: https://github.com/ampproject/amp-wp/issues/5297#issuecomment-683387449

rgomezp commented 4 years ago

Also LGTM. Thanks for the PR @westonruter . This will be released in 2.1.4

SmartandCom commented 4 years ago

@westonruter thanks !