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(android): Ask for POST_NOTIFICATIONS permission if necessary #238

Merged

Conversation

uniquare
Copy link

Description

Adds the following permission to the AndroidManifest.xml:

<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />

Asks the user for permission to post notifications when the plugin is initialized if not already granted by the user.

Related Issue

#215

Motivation and Context

As stated in the issue, Android 13 requires an additional POST_NOTIFICATIONS permission to send non-exempt notifications from an app.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@ricardopetrere
Copy link

🆙

@ricardopetrere
Copy link

While this PR is not accepted, I use its concepts on my fork of the original phonegap-plugin-push plugin (adapting the code, to the limit of my Java skills), and it works fine.

@erisu
Copy link

erisu commented Sep 28, 2023

@uniquare Can you revert the changes you made in www/push.js and then rebase it against the main branch?
Or is it OK if I try to update your branch? I might be able to... but not sure...

@uniquare
Copy link
Author

I've reverted the changes to www/push.js. Do I still need to rebase it (I'm not so familiar with git).

@erisu
Copy link

erisu commented Sep 28, 2023

I am not seeing merge conflicts anymore so I think you dont need to rebase :)

I will merge this in, in a minute.

I will quickly create a PR and merge in the bumping of the cordova-android minimum requirement to >=12.0.0

The requirement bump will be required to go with your changes which added the android.permission.POST_NOTIFICATIONS permission. That permission was introduced in API 33 and Cordova-Android 12.0.0 supports API 33 out of the box..

Older versions of Cordova does not work out of the box.

I want to confirm are you using Cordova-Android 12? or are you changing the sdk build tools with preference flag?

@uniquare uniquare force-pushed the ask-for-post-notifications-permission branch from 50d6d12 to d8d1115 Compare September 28, 2023 10:39
@uniquare
Copy link
Author

Sorry, I rebased in the meantime.

Yes, I am using Cordova-Android 12.0.0.

@erisu erisu merged commit 66138f2 into havesource:master Sep 28, 2023
4 checks passed
@erisu erisu mentioned this pull request Sep 28, 2023
bh-shatz added a commit to talent-rover/cordova-plugin-push that referenced this pull request Oct 9, 2023
Adds a request for permission to post notifications (required for API 33).
Taken from havesource#238
VedantS20 pushed a commit to VedantS20/cordova-plugin-push that referenced this pull request Dec 27, 2023
…esource#238)

* fix(android): Ask for POST_NOTIFICATIONS permission if necessary

---------

Co-authored-by: Daniel Zupan <daniel.zupan@uniquare.com>
mjcctech pushed a commit to mjcctech/cordova-plugin-push that referenced this pull request Jan 3, 2024
…esource#238)

* fix(android): Ask for POST_NOTIFICATIONS permission if necessary

---------

Co-authored-by: Daniel Zupan <daniel.zupan@uniquare.com>
(cherry picked from commit 66138f2)
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.

None yet

3 participants