novuhq / novu

Open-Source Notification Platform. Embeddable Notification Center, E-mail, Push and Slack Integrations.
https://novu.co
Other
35.52k stars 3.92k forks source link

🐛 Bug Report: OneSignal ios_badge_type and ios_badge_count issue #7199

Open L-U-C-K-Y opened 17 hours ago

L-U-C-K-Y commented 17 hours ago

📜 Description

Hi all

We were debugging some things and noticed the following:

In both, the old and new OneSignal API, the payload for ios_badge_type and ios_badge_count is defined as:

New API: https://documentation.onesignal.com/reference/push-notification Old API: https://documentation.onesignal.com/v9.0/reference/push-channel-properties

In the Novu code, the following is used:

We are preparing the MR to send to you and before we finalize it, can you please answer the following questions:

1. What is protected keyCaseObject: Record<string, string> = {?

Does the this.transform function, change any of the keys? Is it related to protected keyCaseObject: Record<string, string> = {

Because in protected keyCaseObject we use ios_badgeType and ios_badgeCount:

packages/providers/src/lib/push/one-signal/one-signal.provider.ts

protected keyCaseObject: Record<string, string> = {
    is_ios: 'isIos',
    is_android: 'isAndroid',
    is_huawei: 'isHuawei',
    is_any_web: 'isAnyWeb',
    is_chrome_web: 'isChromeWeb',
    is_firefox: 'isFirefox',
    is_safari: 'isSafari',
    is_wp_wns: 'isWP_WNS',
    is_adm: 'isAdm',
    is_chrome: 'isChrome',
    ios_badge_type: 'ios_badgeType', // correct ios_badgeType
    ios_badge_count: 'ios_badgeCount', // correct ios_badgeCount
  };
const notification = this.transform(bridgeProviderData, {
      include_player_ids: options.target,
      app_id: this.config.appId,
      headings: { en: options.title },
      contents: { en: options.content },
      subtitle: { en: overrides.subtitle },
      data: options.payload,
      ios_badge_type: 'Increase', // wrong ios_badge_type
      ios_badge_count: 1, // ios_badge_count
      ios_sound: sound,
      android_sound: sound,
      mutable_content: overrides.mutableContent,
      android_channel_id: overrides.channelId,
      small_icon: overrides.icon,
      large_icon: overrides.icon,
      chrome_icon: overrides.icon,
      firefox_icon: overrides.icon,
      ios_category: overrides.categoryId,
    }).body;

2 Duplicate Keys

Why are both keys in the unit test ios_badgeCount and ios_badge_count?

packages/providers/src/lib/push/one-signal/one-signal.provider.spec.ts

expect(data).toEqual({
      include_player_ids: ['tester'],
      app_id: 'test-app-id',
      headings: { en: 'Test' },
      contents: { en: 'Test push' },
      subtitle: {},
      data: { sound: 'test_sound' },
      ios_badge_type: 'Increase',
      ios_badgeCount: 2, // correct key name
      ios_badge_count: 1, // wrong key name
      include_external_user_ids: ['test'],
    });

👟 Reproduction steps

In the codebase

👍 Expected behavior

👎 Actual Behavior with Screenshots

Novu version

Novu SaaS

npm version

No response

node version

No response

📃 Provide any additional context for the Bug.

No response

👀 Have you spent some time to check if this bug has been raised before?

🏢 Have you read the Contributing Guidelines?

Are you willing to submit PR?

Yes I am willing to submit a PR!

linear[bot] commented 17 hours ago

NV-4948 🐛 Bug Report: OneSignal ios_badge_type and ios_badge_count issue