-
Notifications
You must be signed in to change notification settings - Fork 369
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
Remove setRequiresUserConesnt call when there is no value in the manifest #1527
Conversation
…vacyConsent in the manifest
Two tested failed: 1
2
These seem unrelated but rerunning test to confirm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small note on formatting, otherwise the code change itself looks good.
Also I kicked off another run of the tests, they failed due to what looks like flaky tests but did this to confirm that.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @shepherd-l)
OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java, line 886 at r1 (raw file):
// Read the current privacy consent setting from AndroidManifest.xml String requireSetting = bundle.getString("com.onesignal.PrivacyConsent"); if (requireSetting!=null)
Formatting, should be a space before and after operators. Example:
if (requireSetting != null)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR looks good to me now! @shepherd-l you an press the "Merge pull request" button.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @shepherd-l)
OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java, line 886 at r1 (raw file):
Previously, jkasten2 (Josh Kasten) wrote…
Formatting, should be a space before and after operators. Example:
if (requireSetting != null)
Thanks for cleaning this up!
Description
One Line Summary
Remove call to
setRequiresUserConesnt
when there is no value forPrivacyConsent
in the AndroidManifest.Details
Motivation
Removes error log reported in OneSignal/OneSignal-Android-SDK#1520.
Scope
Effects apps that initialize OneSignal with
setRequiresUserConesnt(True)
and have no value forPrivacyConsent
in the AndroidManifest.Testing
Manual testing
Tested by opening the app, app built with Android Studio 2021.1 with a fresh install of the OneSignal example app on a Pixel 6 with Android 12.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is