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): Notifications now stop and clears the channel #114

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

m1aw
Copy link

@m1aw m1aw commented Oct 25, 2018

Platforms affected

Android

What does this PR do?

Ensures notifications stop after playing.
This fixes the issue we had when after 15 notifications, the 16th notification would not play the sound.

What testing has been done on this change?

Use previous version, spam .beep() 16, the 16th notification will not play.
Use this version, the issue is fixed.

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

@janpio
Copy link
Member

janpio commented Oct 30, 2018

no possible side effects?

@m1aw
Copy link
Author

m1aw commented Nov 8, 2018

As far as we could test, no there are no side effects, so we decided to use it in production.
I'm not an expert though, so maybe someone else can take a look at it.

@janpio janpio changed the title Notifications now stop and clears the channel fix(android): Notifications now stop and clears the channel Jul 4, 2019
@janpio janpio closed this Jul 4, 2019
@janpio
Copy link
Member

janpio commented Jul 4, 2019

CI, please rerun tests.

@janpio janpio reopened this Jul 4, 2019
@janpio
Copy link
Member

janpio commented Jul 4, 2019

Android tests are not happy:

1) cordova-plugin-device-tests.tests >> Device Information (window.device) should contain a UUID specification that is a string or a number
  - Expected undefined to be defined.
  - Expected false to be true.

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.

2 participants