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

Fixed some non-working things #44

Closed
wants to merge 3 commits into from
Closed

Fixed some non-working things #44

wants to merge 3 commits into from

Conversation

jachus
Copy link

@jachus jachus commented Jul 12, 2016

No description provided.

@norkunas
Copy link
Owner

@jachus thanks for PR.
What's the reason for moving 'app_id' => $this->api->getConfig()->getApplicationId() to the URL part?

@norkunas norkunas added the bug label Jul 26, 2016
@jachus
Copy link
Author

jachus commented Aug 3, 2016

Hello! app_id is a GET parameter, by the documentation: https://documentation.onesignal.com/docs/players-view-devices

@norkunas
Copy link
Owner

norkunas commented Aug 3, 2016

Hello @jachus, yes I know about that, but it worked for me also when i sent requests with app id in request body. But ok we should use this like documentation states. Could you open a new PR for this? Because this should be fixed also in notifications endpoints.

Also what's with 'offset' => filter_var($offset, FILTER_VALIDATE_INT) ? filter_var could return false now, so i don't understand this change.

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

Successfully merging this pull request may close these issues.

None yet

2 participants