OneSignal / onesignal-node-api

OneSignal Node Client
Other
28 stars 12 forks source link

[Bug]: Having issue with external ids for specific user targeted notification #82

Open sadhan46 opened 1 month ago

sadhan46 commented 1 month ago

What happened?

I'm using node Js to trigger notifications using external Ids even though the notifications are getting triggered for all users. I'm facing issues when triggering device specific notification

  1. external_id is showing up as "null".
  2. the error statement even after using "notification.include_aliases = { "external_id": externalIds }"
CreateNotificationSuccessResponse {
  id: 'ae3093eb-3bda-41a3-b67d-b0281991a08b',
  external_id: null,
  errors: { invalid_aliases: { external_id: [Array] } }
}

Steps to reproduce?

Node version : v21.4.0
One signal version : "@onesignal/node-onesignal": "^5.0.0-alpha-01",

1. Created node js function to trigger notification

const OneSignal = require('@onesignal/node-onesignal');
const { ONE_SIGNAL_CONFIG } = require('../config/app.config');

const configuration = OneSignal.createConfiguration({
    userAuthKey: ONE_SIGNAL_CONFIG.AUTH_KEY,
    restApiKey: ONE_SIGNAL_CONFIG.API_KEY,
});

const client = new OneSignal.DefaultApi(configuration);

// Function to send mobile notifications
async function sendNotification(externalIds, message) {
    try {

        //createNotification
        const notification = new OneSignal.Notification();
        notification.app_id = ONE_SIGNAL_CONFIG.APP_ID;
        notification.name = "external_id";
        notification.contents = {
            en: message
        }

        // required for Huawei
        notification.headings = {
            en: "External Id"
        }
        // Note: Tried passing static external id for testing
        notification.external_id = '66378fb25e9d1a7f26c5252b'

        //Note: passed array of externalIds. For e.g (['66378fb25e9d1a7f26c5252b']
        notification.include_aliases = { "external_id": externalIds }
        notification.target_channel = "push"
        const notificationResponse = await client.createNotification(notification);

        console.log('Notification sent:', notificationResponse);
        return notificationResponse;
    } catch (error) {
        console.error('Error sending notification:', error);
        throw error;
    }
}

module.exports = { sendNotification };

2. Called the function in one of my API

// Send notification
sendNotification([profile._id], 'PROFILE NOTIFICATION')
  .then(() => console.log('Notification sent successfully'))
  .catch(error => console.error('Error sending notification:', error));

3. Called following functions in initState() of my flutter project

  Future<void> initPlatform() async {
    OneSignal.Debug.setLogLevel(OSLogLevel.verbose);

    OneSignal.Debug.setAlertLevel(OSLogLevel.none);
    OneSignal.initialize(AppKeys.oneSignalAppId);
    OneSignal.Notifications.requestPermission(true);

    print(
        'ONE_SIGNAL IDs... ${await OneSignal.User.getOnesignalId()}'); 
    print(
        'ONE_SIGNAL IDs.... External... ${await OneSignal.User.getExternalId()}'); 
  }

  loginOneSignal() async {
    final externalId = await OneSignal.User.getExternalId();
    if (externalId == null) {
      final userId = ref.watch(accountPageProvider).profile?.sId;
      if (userId != null) {
        await OneSignal.logout();
        await OneSignal.login(userId);
      }
    }
  }

What did you expect to happen?

I expected that the notifications will be shown only in the target devices which have logged into oneSignal with custom Id. For e.g. OneSignal.login('custom_id')

Relevant log output

CreateNotificationSuccessResponse {
  id: 'ae3093eb-3bda-41a3-b67d-b0281991a08b',
  external_id: null,
  errors: { invalid_aliases: { external_id: [Array] } }
}

Code of Conduct

jpike88 commented 1 month ago

I have the same problem on 5.0.0-alpha-01. @nan-li @sadhan46 we want to migrate to the new user model, but this is a critical, basic use case that is failing for us.

const pushPayload = {
        username: 'blah@blah.com',
        userID: 'example_user_id',
        data: {
            custom: {
                // some stuff here
            },
            title: 'Reminder',
            body: 'Please ensure you keep your apps up to date.',
        },
        whiteLabelKey: 'blah',
    };

    const notification = new Notification();
    notification;
    notification.app_id = 'APP_ID_HERE';

    notification.contents = {
        en: pushPayload.data.body,
    };
    notification.headings = {
        en: pushPayload.data.title,
    };
    notification.data = pushPayload.data.custom;
    // biome-ignore lint/style/useNamingConvention: onesignal api
    notification.include_aliases = { external_id: [pushPayload.userID] };
    notification.target_channel = 'push';

    const configuration = createConfiguration({
        restApiKey: 'REST_API_KEY_HERE',
    });
    const testOneSignalClient = new DefaultApi(configuration);
    const result = await testOneSignalClient.createNotification(notification);

I get the same issue:

CreateNotificationSuccessResponse {
  id: '',
  errors: { invalid_aliases: { external_id: ['example_user_id'] } }
}
nan-li commented 1 month ago

Hi @sadhan46 and @jpike88 thank you for reporting, I will try to reproduce this.

nan-li commented 1 month ago

I reproduced it when the external_id I provided has multiple subscriptions, some of which are invalid or opted out.

Here is a snippet from our Create Notification REST API describing sample responses when using include_aliases.

Does this clarify the responses you are seeing?

// If an "id" is provided, then at least 1 subscription was sent the message.
// First "external_id" reference is to the Idempotent Key: https://documentation.onesignal.com/docs/idempotent-notification-requests
// "invalid_aliases" correspond to how many subscriptions for the user are opted-out of receiving messages for this channel.
// In this example, at least 1 subscription was sent the message and the user with this "external_id" had 3 opted-out subscriptions.
{
    "id": "bfc9ea0a-8488-441d-9208-7154794d426a",
    "external_id": null,
    "errors": {
        "invalid_aliases": {
            "external_id": [
                "123456789",
                "123456789",
                "123456789"
            ]
        }
    }
}

// If an "id" is not provided, then the user has no subscriptions opted-in to receive messages for the channel.
// In this example, the user has 2 subscriptions for this channel and both are opted-out from receiving messages. 
{
    "id": "",
    "errors": {
        "invalid_aliases": {
            "external_id": [
                "123456789",
                "123456789"
            ]
        }
    }
}
jpike88 commented 1 month ago

That seems like a pretty complex for what should be a drop in replacement for the old external_id property in the v2 library. Why can't it behave the same way? I specify external id or ids, and I want the notification to be registered. Whatever subscriptions may or may not be registered with the external id should not be relevant.

nan-li commented 1 month ago

Hi @jpike88, you can just check for the id of the notification in the response, and ignore the additional information. Some users have use cases for that information.

Using the previous include_external_user_ids property actually did return similar information, but the OneSignal Node SDK just did not parse it correctly and did not return it in the response object.

jpike88 commented 1 month ago

check for the id of the notification in the response

CreateNotificationSuccessResponse {
  id: '',
  errors: { invalid_aliases: { external_id: ['example_user_id'] } }
}

It's an empty string, so I don't know how to interpret that.

I reproduced it when the external_id I provided has multiple subscriptions, some of which are invalid or opted out.

Where's the test suite that would have picked this up? That seems like a common case.

nan-li commented 1 month ago

It's an empty string, so I don't know how to interpret that.

That means no notifications were created and it could be due to any of these reasons:

  1. There is no user with an external ID of example_user_id
  2. The user with an external ID of example_user_id has no active subscriptions. That means those subscriptions are deleted, opted out, or turned off notifications permission.

Can you confirm that user with an external ID of example_user_id has active push subscriptions?

You can also try hitting our Create Notification endpoint directly: https://documentation.onesignal.com/reference/create-notification.


I reproduced it when the external_id I provided has multiple subscriptions, some of which are invalid or opted out. Where's the test suite that would have picked this up? That seems like a common case.

Unclear word choice on my part, the reproduction is expected.

If a user with external ID of example_user_id has 5 subscriptions, and 2 are active, this user will be noted in invalid_aliases 3 times for the 3 subscriptions that are inactive. The 2 active subscriptions will should receive the notification.