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 Report: OneSignal ios_badge_type and ios_badge_count issue #7199

Open
2 tasks done
L-U-C-K-Y opened this issue Dec 3, 2024 · 1 comment · May be fixed by #7273
Open
2 tasks done

🐛 Bug Report: OneSignal ios_badge_type and ios_badge_count issue #7199

L-U-C-K-Y opened this issue Dec 3, 2024 · 1 comment · May be fixed by #7273
Assignees
Labels

Comments

@L-U-C-K-Y
Copy link
Contributor

📜 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:

  • ios_badgeType
  • ios_badgeCount

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:

  • ios_badge_type
  • ios_badge_count

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

  • ios_badgeType
  • ios_badgeCount

👎 Actual Behavior with Screenshots

  • ios_badge_type
  • ios_badge_count

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?

  • I checked and didn't find a similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to submit PR?

Yes I am willing to submit a PR!

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 a pull request may close this issue.

2 participants