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]: Having issue with external ids for specific user targeted notification #82

Open
1 task done
sadhan46 opened this issue May 7, 2024 · 7 comments
Open
1 task done
Labels
bug Something isn't working

Comments

@sadhan46
Copy link

sadhan46 commented May 7, 2024

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

  • I agree to follow this project's Code of Conduct
@sadhan46 sadhan46 added the bug Something isn't working label May 7, 2024
@jpike88
Copy link

jpike88 commented May 9, 2024

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: '[email protected]',
		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
Copy link
Contributor

nan-li commented May 9, 2024

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

@nan-li
Copy link
Contributor

nan-li commented May 9, 2024

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
Copy link

jpike88 commented May 10, 2024

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
Copy link
Contributor

nan-li commented May 14, 2024

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
Copy link

jpike88 commented May 17, 2024

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
Copy link
Contributor

nan-li commented May 17, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants