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

[Bug]: Unable to push notifications to users within segment #18

Open
1 task done
iAmWillShepherd opened this issue Jun 24, 2022 · 3 comments
Open
1 task done
Labels

Comments

@iAmWillShepherd
Copy link

What happened?

I built the following sample app to push notifications to all users within a segment, but unfortunately, I was unable to get it to work.

require 'dotenv/load'
require 'time'
require 'onesignal'

rest_api_key = ENV['ONESIGNAL_REST_API_KEY']
app_id = ENV['ONESIGNAL_APP_ID']

OneSignal.configure do |config|
  config.app_key = rest_api_key
end

api_instance = OneSignal::DefaultApi.new

begin
  notification = OneSignal::Notification.new({ app_id: app_id })
  notification.contents = OneSignal::StringMap.new({ "en": 'English Message' })
  notification.included_segments = ['Include_segment']
  p notification

  notification_response = api_instance.create_notification(notification)
  notification_id = notification_response.id
  p notification_response
rescue OneSignal::ApiError => e
  puts "Error when calling DefaultApi->create_notification: #{e}"
end

FWIW, @jmadler and I did a little debugging to confirm that my segment did indeed have users who were subscribed to push, but still got the same error. I also tried several different apps and other segments and got the same results.

Steps to reproduce?

1. Change the `rest_api_key` and `app_id` to an app that makes use of segments
2. Run the script I shared
3. Note that you receive `All included players are not subscribed` as an error message

What did you expect to happen?

I expected a push notification to be sent to my device.

Relevant log output

#<OneSignal::Notification:0x00000001072d9440 @is_ios=true, @app_id="8152db8f-1417-48a4-b8db-264a744b87a3", @contents=#<OneSignal::StringMap:0x00000001072d9170 @en="English Message">, @included_segments=["Subscribed Users"]>
#<OneSignal::InlineResponse200:0x0000000107430bb8 @id="", @recipients=0, @errors=["All included players are not subscribed"]>

Code of Conduct

  • I agree to follow this project's Code of Conduct
@iAmWillShepherd iAmWillShepherd changed the title [Bug]: [Bug]: Unable to push notifications to users within segment Jun 24, 2022
@kesheshyan
Copy link
Contributor

Hey @iAmWillShepherd, thanks for reporting this. It seems you also need to specify:

notification.is_chrome_web = true
notification.is_any_web= true

I'm not sure why but that is the way our backend works.

@iAmWillShepherd
Copy link
Author

Hmm, that is interesting 🤔

I think we should keep this issue open with the action item to update the docs for this project to highlight these quirks. What do you think?

@jkasten2
Copy link
Member

jkasten2 commented Jun 28, 2022

I think this should be documented as well, since these are not considered required fields today by this SDK's defined API.

Also maybe we can enforce at least one is_ value is set to true and report that as an error before attempting to make the network call?
Or maybe even better, the REST API itself can return a more helpful error when this happens.

  • Background on how the REST API works today: If you omit all is* properties today from the REST API it assumes all are true, however if you pass any is* flags all others are defaulted to false.

Lastly, in a 2.0.0 major release, we could make all these true by default (or omit them to give the same effected as noted above)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants