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

Fix Andorid API 31 Pending Intent Crash #41

Closed

Conversation

masterwok
Copy link
Contributor

@masterwok masterwok commented Apr 27, 2022

This pull request fixes a crash that occurs when receiving a push notification on Android API 31+ (#40)

This crash was due to the Android Iterable SDK not setting mutability on a PendingIntent.

The fix was to bump the Android Iterable SDK to version 3.4.2 as discussed here: Iterable/iterable-android-sdk#430

Other changes:

  • Bumped Kotlin version to 1.6.21 (had to bump to support API 31 so bumped to latest)
  • flutter pub get implicitly upgraded other packages

I wasn't sure what versioning strategy you use so I didn't bump the iterable-flutter package version.

Below is a video demonstrating the fix:

Screen.Recording.2022-04-27.at.11.19.14.AM.mov

@justinelliss-coinme
Copy link

Thanks man - this resolves an issue we were having

Copy link
Contributor

@Santi92 Santi92 left a comment

Choose a reason for hiding this comment

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

Excellent for your contribution and solving this bug in Android API 31. 🎊 🎸

I left you some comments about changes that don't go with this fix.

Comment on lines 90 to 97
version: "0.12.11"
material_color_utilities:
dependency: transitive
description:
name: material_color_utilities
url: "https://pub.dartlang.org"
source: hosted
version: "0.1.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand how there are changes in the pubspec.lock but there are no changes in the pubspec.yaml, WDYT ?

Or it could be that you missed it in the PR by mistake, or if you made a change to the example to improve it, it would be great if you did it in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran flutter pub get in the root of the project and these packages were implicitly updated. I didn't change anything in the example project.

 jonathantrowbridge@Jonathans-MacBook-Pro-2  ~/projects/iterable-flutter   master  git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
 jonathantrowbridge@Jonathans-MacBook-Pro-2  ~/projects/iterable-flutter   master  flutter pub get
Running "flutter pub get" in iterable-flutter...                   909ms
Running "flutter pub get" in example...                          1,089ms
 jonathantrowbridge@Jonathans-MacBook-Pro-2  ~/projects/iterable-flutter   master ●  git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   example/pubspec.lock
        modified:   pubspec.lock

no changes added to commit (use "git add" and/or "git commit -a")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just searched around a bit and it looks like dart.dev recommends against committing pubspec.dart for library packages. More information:

From the documentation:

pubspec.lock

The pubspec.lock file is a special case, similar to Ruby’s Gemfile.lock.

For library packages, don’t commit the pubspec.lock file. Regenerating the pubspec.lock file lets you test your >package against the latest compatible versions of its dependencies.

For application packages, we recommend that you commit the pubspec.lock file. Saving pubspec.lock ensures that everyone working on the app uses the exact same versions._

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Santi92 how would you like me to handle the pubspec.lock file? Once we get this merged down I have another PR I'd like to submit for review. This PR adds support for tracking event data on iOS and Android: masterwok#1

pubspec.lock Outdated
Comment on lines 76 to 83
version: "0.12.11"
material_color_utilities:
dependency: transitive
description:
name: material_color_utilities
url: "https://pub.dartlang.org"
source: hosted
version: "0.1.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. changes in the pubspec.lock but there are no changes in the pubspec.yaml

pubspec.lock Outdated
Comment on lines 76 to 83
version: "0.12.11"
material_color_utilities:
dependency: transitive
description:
name: material_color_utilities
url: "https://pub.dartlang.org"
source: hosted
version: "0.1.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. changes in the pubspec.lock but there are no changes in the pubspec.yaml

Copy link
Contributor

@danielgomezrico danielgomezrico left a comment

Choose a reason for hiding this comment

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

@Masterwork thanks for the PR! I left some minor questions, and can you please do a rebase with master? I made a fix to see if your PR works fine for android on #44.

@@ -2,7 +2,7 @@ group 'com.lahaus.iterable_flutter'
version '1.0-SNAPSHOT'

buildscript {
ext.kotlin_version = '1.3.50'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to update kotlin for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding, I believe so. When I bumped compileSdkVersion 31 without updating the Kotlin version it resulted in the following error:

* What went wrong:
Execution failed for task ':iterable_flutter:compileDebugKotlin'.
> Compilation error. See log for more details

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 11s

┌─ Flutter Fix ────────────────────────────────────────────────────────────────────────────────┐
│ [!] Your project requires a newer version of the Kotlin Gradle plugin.                       │
│ Find the latest version on https://kotlinlang.org/docs/gradle.html#plugin-and-versions, then │
│ update /Users/jt/projects/iterable-flutter/example/android/build.gradle:                     │
│ ext.kotlin_version = '<latest-version>'                                                      │
└──────────────────────────────────────────────────────────────────────────────────────────────┘

Because of this, I decided to bump to latest. I just noticed one thing however, we should probably bump targetSdkVersion to match the compileSdkVersion. I'm going to push up that change now.

@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.3.50'
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@masterwok masterwok closed this May 3, 2022
@masterwok masterwok force-pushed the bugfix/android-api-31-crash branch from 8bee2bb to 00b488d Compare May 3, 2022 13:42
@masterwok
Copy link
Contributor Author

@Santi92 @Santi92 sorry, had to close this PR. Going to open again. I accidentally pushed something I didn't mean to 🙃 . Sorry about that.

@masterwok
Copy link
Contributor Author

Reopened under due to mistake: #46

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.

4 participants