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

[PAY-2142] Fix Deeplinks Not Routing From Braze #1775

Merged
merged 9 commits into from Jan 25, 2023

Conversation

msadoon
Copy link
Contributor

@msadoon msadoon commented Jan 19, 2023

πŸ“² What

Marketing reporting that deeplinks failed to route in-app after our upgrade of Appboy-iOS-SDK to Braze iOS SDK.
We went a few versions forward there from 4.3.2 to 5.5.0, and didn't fully test in-app deeplinking from PN's sent with Braze.

πŸ€” Why

Marketing campaigns blocked from being effective if content cannot be directed for all supported deeplinks.

πŸ›  How

Turns out we never handled the Braze deeplink in our existing implemention where we decode the payload from Braze. AppDelegateViewModel line 329 PushEnvelope has a aps case that contains an alert property which matches what's coming from Braze's payload, but the actual URL in braze's payload was a separate key called ab_uri. This key is not decoded.

So...I'm not sure how this ever worked, unless it was by accident.

Now there is a BrazePushEnvelope that handles all ab_uri's coming from the system userNotificationCenter(didRecieve, withCompletionhandler) delegate method.

there are some .pbxproj settings related to code-signing that are okay to have in because they specify the provisioning which is required by our CI to match the build type (alpha/beta/debug) with the profiles found on ios-certificates

Note:
In-app notifications are still not working as expected. The issue is that ABKInAppMessageControllerDelegate is not being triggered, but the in-app notification is coming in as a PN through UNUserNotificationCenterDelegate. Suggestion for finding the fix here is to remove Segment-Appboy-SDK, integrate with Appboy_SDK not Braze_iOS_SDK then replace Appboy_SDK with Segment-Appboy-SDK.

Made a separate ticket (NTV-647) for this.

  1. Start here. Once it's complete, test pn's work and integrate for In-app notifications.
  2. Troubleshooting
  3. Sample with vanilla In-app notifications (no push notifications)

πŸ‘€ See

Before πŸ›

RPReplay_Final1674488350.MP4

After πŸ¦‹

RPReplay_Final1674488257.MP4

βœ… Acceptance criteria

  • Ensure deeplinks route to their destination in app. Test a few from this list. That's also the list that shows how to set up the project to accept deeplinks.

⏰ TODO

  • Ensure first install deeplinks appear on device, right now they fail to appear from Braze.
  • Ensure base.xcconfig and .xcodeproj related changes are not persisted unless they are file adds.
  • Ensure in-app messages still come through.

@msadoon msadoon self-assigned this Jan 19, 2023
@msadoon msadoon added the bug label Jan 19, 2023
@msadoon msadoon added this to the release-5.6.1 milestone Jan 19, 2023
@msadoon msadoon changed the title [PAY-2142] Deeplinks Not Routing From Braze [PAY-2142] Fix Deeplinks Not Routing From Braze Jan 19, 2023
@codecov
Copy link

codecov bot commented Jan 19, 2023

Codecov Report

Merging #1775 (7d37965) into main (6d59c58) will decrease coverage by 0.01%.
The diff coverage is 65.21%.

@@            Coverage Diff             @@
##             main    #1775      +/-   ##
==========================================
- Coverage   85.24%   85.23%   -0.01%     
==========================================
  Files        1276     1277       +1     
  Lines      116682   116695      +13     
  Branches    30779    30786       +7     
==========================================
+ Hits        99462    99470       +8     
- Misses      16146    16150       +4     
- Partials     1074     1075       +1     
Impacted Files Coverage Ξ”
KsApi/models/BrazePushEnvelope.swift 0.00% <0.00%> (ΓΈ)
Library/Tracking/Segment.swift 0.00% <0.00%> (ΓΈ)
Kickstarter-iOS/AppDelegateViewModel.swift 91.57% <90.00%> (-0.03%) ⬇️
Kickstarter-iOS/AppDelegateViewModelTests.swift 98.96% <100.00%> (ΓΈ)

πŸ“£ We’re building smart automated test selection to slash your CI/CD build times. Learn more

Those are still not showing as intended. It's still not clear why the delegate methods for ABKInAppMessageControllerDelegate are not triggered. The delegate property of `Appboy.sharedInstance()?.inAppMessageController` is set. The in-app notifications come in as push notifications.
@msadoon msadoon marked this pull request as ready for review January 23, 2023 22:59
@msadoon
Copy link
Contributor Author

msadoon commented Jan 23, 2023

SwiftFormat found issues:

File Rules
KsApi/lib/KsApiNotification.swift:5:1 warning: (wrap) Wrap lines that exceed the specified maximum width.

Generated by 🚫 Danger

Weird formatting issue, not sure why it's putting it on a newline.

Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

@msadoon Code looks good to me and the app runs normally. Wasn't able to test the deep-linking though because Braze isn't identifying my account.

@msadoon
Copy link
Contributor Author

msadoon commented Jan 24, 2023

@msadoon Code looks good to me and the app runs normally. Wasn't able to test the deep-linking though because Braze isn't identifying my account.

Noted, I can't seem to register a staging only Braze user. If you're okay with approving I can run through some deeplink tests omo, or pair to show you that deeplinks work.

@scottkicks
Copy link
Contributor

@msadoon that'd be great. thanks. just throw some time on my calendar whenever.

@msadoon msadoon merged commit 36dffc5 into main Jan 25, 2023
@msadoon msadoon deleted the fix/braze-pns-not-deeplinking branch January 25, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants