search

nutritionfactsorg / daily-dozen-android

Keep track of the foods that Dr. Greger recommends in his NYT's best-selling book, How Not to Die with this Android app
https://play.google.com/store/apps/details?id=org.nutritionfacts.dailydozen&hl=en
Other
274 stars 95 forks source link

No way to enable notifications on v22 #219

Closed k4rtik closed 1 year ago

k4rtik commented 1 year ago

It seems with the latest Dec 23 update, the app cannot send reminder notifications. Something is not used correctly in the Android SDK upgraded to in that update.

In the doc for notification permissions, I see that POST_NOTIFICATIONS needs to be declared which seems to be missing in the manifest.

I am on Pixel 7 and Android 13.

slavick commented 1 year ago

I'll look into it. Oh the joys of Android development. I have a Pixel 3a and a Galaxy Note 10 Plus, both of which are still perfectly capable phones and yet each are no longer supported and stuck on Android 12. I guess my next phone is the emulator. Happy New Year!

k4rtik commented 1 year ago

Happy New Year to you too, let me know if you need me to help test on my device.

slavick commented 1 year ago

Update: I haven't had much time to work on this and I'll be on vacation for the next two weeks. I won't be getting around to working on this until after.

So far it isn't as simple as just adding the permission to the manifest. The flow for asking the user for the permission also needs to be implemented.

slavick commented 1 year ago

I've created a draft PR for fixing this issue: https://github.com/nutritionfactsorg/daily-dozen-android/pull/227

@k4rtik Would you be willing to test my fix for this issue? It would require installing an apk file directly. I don't have a physical Android 13 device, so I've only been able to test using the emulator.

k4rtik commented 1 year ago

Yes, I can do that. Where can I download the APK?

On Sun, Mar 19, 2023 at 4:23 PM John Slavick @.***> wrote:

I've created a draft PR for fixing this issue: #227 https://github.com/nutritionfactsorg/daily-dozen-android/pull/227

@k4rtik https://github.com/k4rtik Would you be willing to test my fix for this issue? It would require installing an apk file directly. I don't have a physical Android 13 device, so I've only been able to test using the emulator.

— Reply to this email directly, view it on GitHub https://github.com/nutritionfactsorg/daily-dozen-android/issues/219#issuecomment-1475403508, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC3MRA5G4F7OK6EJUG7OSTW452LVANCNFSM6AAAAAATNSHH3A . You are receiving this because you were mentioned.Message ID: @.***>

-- Kartik

slavick commented 1 year ago

@k4rtik I'll email you at your github email address.

slavick commented 1 year ago

Closing this issue as the bug has been fixed and included in the v22.1 release.

@k4rtik I hope you will reconsider your 1 star review mentioning this bug. Thanks for the bug report and helping with the fix!

k4rtik commented 1 year ago

For sure, thanks for the fix!

On Sat, Mar 25, 2023, 10:47 John Slavick @.***> wrote:

Closing this issue as the bug has been fixed.

@k4rtik https://github.com/k4rtik I hope you will reconsider your 1 star review mentioning this bug. Thanks for the bug report and helping with the fix!

— Reply to this email directly, view it on GitHub https://github.com/nutritionfactsorg/daily-dozen-android/issues/219#issuecomment-1483854824, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC3MRDU7Y2ND2QZ5MOLJMLW54HRJANCNFSM6AAAAAATNSHH3A . You are receiving this because you were mentioned.Message ID: @.***>