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

[CHNL-12942] Create a hook to reset badge counts #235

Merged
merged 13 commits into from
Nov 26, 2024

Conversation

belleklaviyo
Copy link
Contributor

@belleklaviyo belleklaviyo commented Nov 12, 2024

Description

These changes introduce a new setter setBadgeCounton the SDK. By default this setter will be called on every foreground event to clear it/set the badge count to 0 and controlled by the Klaviyo_badge_autoclearing flag in the plist, configurable by the developer. The badge count will also be synced with the cached count we track in the App Group User Defaults (to be explained in the updated README instructions). If devs so want to disable the autoclearing and call the setter themselves, they can do so like KlaviyoSDK().setBadgeCount() after initialization.

Small note: Are we ok with the name setBadgeCount, there is the method on UNUserNotificaitonCenter with the same name.

Check List

  • Are you changing anything with the public API?
  • Have you tested this change on real device?
  • Are your changes backwards compatible with previous SDK Versions?
  • Have you added unit test coverage for your changes?
  • Have you verified that your changes are compatible with all the operating system version this SDK currently supports?

Manual Test Plan

Note: this test plan worked fine on device, but did not auto clear on simulator

Default behavior

  1. Add Klaviyo_badge_autoclearing to the target app Info.plist as a Boolean and set it to YES
  2. Send some notification from Push Notification Console to populate the badge count (i.e. badge = 3 in payload) with the app in the background and observe badge of 3
  3. Open the app
  4. Close the app and observe badge cleared to 0

Default behavior turned off

  1. Set in the Info.plist the Klaviyo_badge_autoclearing key is set to NO
  2. Send some notification from Push Notification Console to populate the badge count (i.e. badge = 3 in payload) with the app in the background and observe badge of 3
  3. Open the app
  4. Close the app and observe badge is still 3

Custom call with default clearing turned off

  1. Add KlaviyoSDK().setBadgeCount(0) to some button or trigger in the app
  2. Send some notification from Push Notification Console to populate the badge count (i.e. badge = 3 in payload)
  3. Observe when reset method is triggered the badge count is reset to 0

Supporting Materials

@belleklaviyo belleklaviyo requested a review from a team as a code owner November 12, 2024 15:12
Copy link

@jasper-hsu-kyvo jasper-hsu-kyvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, however adding Andrew to the review because he knows way more about the SDKs than I do (so won't approve sorry) - also looks like you need to rerun pre-commit though and have a formatting issue.

Copy link
Contributor

@ab1470 ab1470 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, why do we need to persist the current badge number to UserDefaults?

Sources/KlaviyoSwift/Klaviyo.swift Outdated Show resolved Hide resolved
@ab1470
Copy link
Contributor

ab1470 commented Nov 12, 2024

Also, I think we'll want to get have some unit tests for these changes

@belleklaviyo
Copy link
Contributor Author

Just wondering, why do we need to persist the current badge number to UserDefaults?

this is necessary for how we are going to keep track and increment the badge count, details here but still some things up in the air, might convert to a draft PR in the meantime

@belleklaviyo belleklaviyo marked this pull request as draft November 12, 2024 20:51
@belleklaviyo belleklaviyo marked this pull request as ready for review November 20, 2024 14:42
Copy link
Contributor

@ajaysubra ajaysubra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor comments from Noah and I.

Sources/KlaviyoSwift/Klaviyo.swift Outdated Show resolved Hide resolved
Sources/KlaviyoSwift/StateManagement/StateManagement.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@ab1470 ab1470 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice work.

Sources/KlaviyoCore/KlaviyoEnvironment.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@ajaysubra ajaysubra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit picks else LGTM.

getBackgroundSetting: { .create(from: UIApplication.shared.backgroundRefreshStatus)
},
getBadgeAutoClearingSetting: {
Bundle.main.object(forInfoDictionaryKey: "Klaviyo_badge_autoclearing") as? Bool ?? true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are unable to get the plist value we assume that they want to auto clear? I think this is fine but might be good to callout in the readme.

Comment on lines 86 to 87
/// Sets the badge number on the application icon. Syncs with the persisted count
/// stored in the User Defaults suite set up with the App Group
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we mention something about how/why this value is used?

@belleklaviyo belleklaviyo merged commit b283644 into master Nov 26, 2024
14 checks passed
@belleklaviyo belleklaviyo deleted the bl/reset-badge-hook branch November 26, 2024 19:17
belleklaviyo added a commit that referenced this pull request Jan 2, 2025
[CHNL-12942] Create a hook to reset badge counts
belleklaviyo added a commit that referenced this pull request Jan 10, 2025
[CHNL-12942] Create a hook to reset badge counts
belleklaviyo added a commit that referenced this pull request Jan 10, 2025
[CHNL-12942] Create a hook to reset badge counts
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

Successfully merging this pull request may close these issues.

5 participants