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

Upgrade the urbanairship-react-native package #3508

Merged
merged 3 commits into from
Jun 10, 2021
Merged

Conversation

Jag96
Copy link
Contributor

@Jag96 Jag96 commented Jun 10, 2021

Details

This PR updates the urbanairship-react-native package to fix an issue where removing all listeners (PushNotification.deregister) throws an error in the old package.

Fixed Issues

Fixes #3503

Tests

  1. Sign into an account and then sign out, confirm the apps don't crash

While testing this, I noticed that the TestFlight build sent push notifications properly, but when building to a physical device (iOS since I don't have an Android) the push notifications never came through. I confirmed that the same behavior happens when building to my device using the same version that is on the TestFlight (where pushes are working fine), so this may have to be fully confirmed when the build is on the TF. cc @roryabraham and @sketchydroide to confirm since you both have tested this before.

QA Steps

On the testflight build, sign into an account on iOS and Android then background the app. On another account, send a message to that account and confirm you receive a push notification.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

@Jag96 Jag96 self-assigned this Jun 10, 2021
@Jag96 Jag96 requested a review from a team as a code owner June 10, 2021 03:27
@MelvinBot MelvinBot requested review from HorusGoul and TomatoToaster and removed request for a team June 10, 2021 03:27
Copy link
Contributor

@Julesssss Julesssss left a comment

Choose a reason for hiding this comment

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

Tests well for me on iOS simulator and Android emulator. I noticed that we seem to get stuck when logging back in after logging out. But a refresh resolves this and [this was not introduced by this PR](Endless loading spinner appears when re login
).

(I'm reviewing as we need to unblock deployBlockers for our N5 build)

@Julesssss Julesssss removed the request for review from sketchydroide June 10, 2021 16:24
@Julesssss
Copy link
Contributor

Removing @sketchydroide as he's off today. Would one of the remaining reviewers be able to review this today? It's one of a few issues blocking the N5 build deployment.

@TomatoToaster TomatoToaster merged commit 2290e28 into main Jun 10, 2021
@TomatoToaster TomatoToaster deleted the joe-upgrade-ua branch June 10, 2021 16:27
@Julesssss
Copy link
Contributor

Julesssss commented Jun 10, 2021

Thanks Amal!!!

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.66-13🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@kavimuru
Copy link

Android - Notification not received if app is backgrounded

Expected Result:

User receives push notification

Actual Result:

No push notification received

Actions Performed:

  1. Launch the app and login
  2. Background the app.
  3. From another account, send a message to this account

Platform:

Android ✔️

Build:

1.0.67-0

Notes/Images/Video:

Bug5107452_screen-20210610-194541.mp4

@Julesssss
Copy link
Contributor

The above issue has been moved to it's own issue: #3542 (comment)

@isagoico
Copy link

Checking the PR off the checklist since the bug will be addressed on a different issue.

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.68-4🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

Android - App crashes when logging out
6 participants