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

Notification not working with new Ionic Push #181

Closed
SarasArya opened this issue Nov 19, 2015 · 10 comments
Closed

Notification not working with new Ionic Push #181

SarasArya opened this issue Nov 19, 2015 · 10 comments

Comments

@SarasArya
Copy link

I was actually trying to run the latest Node-gcm with Ionic Push Library. Seems to me that if I just paste var message = new gcm.Message(); the notification is received but without the payload and if I try and add the addNotification() field the notification is not delivered.
Server side code

message.addData('message', "This is my notification");
message.addNotification({
  title: 'Alert!!!',
  body: 'Abnormal data access',
  icon: 'icon',
  message : 'This is my last try'
 });
var sender = new gcm.Sender('SENDERS_API_KEY'); 

var regTokens =  [];
regTokens.push(req.body.regId);

    sender.send(message, { registrationTokens: regTokens }, function (err, result) {
        if (err) {
            console.error(err);
        }
        else {

             console.log(result);
             res.status(200);
             res.set('Content-Type', 'application/json');
             res.send(result);
        }
    });   

front end code

app.run(function($ionicPlatform, $ionicPush,$http) {
$ionicPlatform.ready(function() {
var push = new Ionic.Push({
"onNotification": function(notification) {
var payload = notification.payload;
console.log(payload);
console.log(notification);
console.log("notification received");
},
"onRegister": function(data) {
console.log(data.token);
var reg = {
regId : data.token
 };
$http.post('http://app.metaiot.com/sendPush', reg)
.then(function(response){
 console.log(response);
 }, function (error){
console.log(error);
 }); 
}
});

So I wanted to confirm first. Is this an issue from the node-gcm end or Ionic End? Can somebody implement it and let us know? becuase I have boiled it down to the very fact that it's just and issue with addNotification function.

@eladnava
Copy link
Collaborator

@SarasArya I don't think that you should be adding a message: key to the addNotification() call. Can you try without it?

  message : 'This is my last try'

Also, check out this issue which reports similar behavior with PhoneGap:

You are right,
Its not node gcm issue. However i debugged and was able to solve it. Just incase anyone stumbles here,
The Phonegap plugin needs the 3 notification fields. phonegap plugin reads the title and the icon from the "notification" object however the message needs to be set in the "data" object.

@hypesystem
Copy link
Collaborator

As @eladnava points out, there are generally some issues with a lot of client-side libraries not yet supporting the notification field. It was added with Android M (quite recently).

It all else fails, I would recommend you stick to the data field and build and show the notifications client-side (instead of relying on automatic showing).

@SarasArya
Copy link
Author

Sorry for the issue it was something from ionic side.... They had given everything in their docs just for one thing. They were initialising a constructor with 2 parameter and the docs asked us to set only one. I have reported the issue. Lets hope someone will find it useful if it's there Keep up the good work. I am closing the issue. Thanks @hypesystem and @eladnava

@eladnava
Copy link
Collaborator

@SarasArya Can you please link to your report of this bug to Ionic so we can list it in the Compatibility Documentation? (#155)

@hypesystem
Copy link
Collaborator

As I read it, the Ionic implementation actually worked, but something was documented poorly. It seems this was a false negative, not a bug.

@eladnava
Copy link
Collaborator

eladnava commented Jan 2, 2016

They were initialising a constructor with 2 parameter and the docs asked us to set only one

@hypesystem I guess we'll never know unless @SarasArya responds.

@SarasArya
Copy link
Author

@hypesystem This wasn't a bug. Just poor documentation. I don't know how it worked for others. @eladnava How do I link my report to Ionic?

@eladnava
Copy link
Collaborator

eladnava commented Jan 2, 2016

@SarasArya You stated this:

They were initialising a constructor with 2 parameter and the docs asked us to set only one. I have reported the issue.

How did you report the issue? GitHub? E-mail?

@SarasArya
Copy link
Author

I emailed them... They have a suggest edit in the docs. I gave my suggestion there. That's that

@eladnava
Copy link
Collaborator

eladnava commented Jan 3, 2016

@SarasArya Cool, do you know if they updated the docs to fix the mistake?

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

No branches or pull requests

3 participants