Upgrade to SDK 34 #5790

rohit9625 · 2024-08-24T16:23:13Z

Description (required)

Note:- Please test these changes and give your feedback. From this change, the normal gallery picker prompts the user to the photo picker, which is not the desired behavior and it requires major changes. This PR is testing only for Now. Don't consider it to be production-ready.

What changes did you make and why?
1. Add functionality to work with Partial Access on API >= 34

This includes adding a new UI component to manage partially accessed photos.
Add Jetpack Compose to project:-Add dependencies to create that new UI component using Jetpack Compose in ComposeView.
Perform some refactoring on related classes and methods.
Add READ_MEDIA_VISUAL_USER_SELECTED permission in Manifest File and limit some permissions to a specific SDK.

2. Upgrade AGP to latest and targetSDK/compileSDK to 34

3. Minor refactoring

Some changes are not related to this migration but while exploring I found some changes that were needed
Migrating to Partial Access was causing some crashes for specific actions. That's why I need to make some changes to files under the Custom Selector package.
Reduce compiler warnings by adding final keywords to some files(that I saw while exploring)

4. Replace deprecated circular progress bar

Remove the deprecated progress bar from activity_quiz_result and fragment_achievements and add new material circular progress bar

Tests performed (required)

Tested ProdDebug on Samsung A14(API 34), Realme Narzo 50(API 33), Pixel 5 Emulator(API 29), and Pixel 6 Emulator(API 31).

Screenshots (for UI changes only)

New UI Component

For optimized code only

Additionally, add Jetpack Compose to the project

DescriptionActivity should not be exposed

Additionally, add final keywords to reduce compiler warnings

Implement using composeView

nicolas-raoul · 2024-08-25T04:08:40Z

I get Could not find com.dinuscxj:circleprogressbar:1.1.1.

This repository/branch solves the problem, so would you mind pulling from it? [email protected]:kanahia1/apps-android-commons.git:issue5583
Or just the circleprogressbar parts if you prefer, but pulling the whole branch would be more efficient.

Thanks a lot! :-)

For optimized code only

Additionally, add Jetpack Compose to the project

DescriptionActivity should not be exposed

Additionally, add final keywords to reduce compiler warnings

Implement using composeView

…ommons into upgrade

rohit9625 · 2024-08-25T04:40:54Z

I get Could not find com.dinuscxj:circleprogressbar:1.1.1.

This repository/branch solves the problem, so would you mind pulling from it? [email protected]:kanahia1/apps-android-commons.git:issue5583 Or just the circleprogressbar parts if you prefer, but pulling the whole branch would be more efficient.

Thanks a lot! :-)

That branch has so many changes and I looked at the progress bar part. The code was removed and commented out as I did on my local machine. What about I add a new progress bar using Jetpack and push the latest changes or wait for kanahia's PR to get merged?

RitikaPahwa4444

The new UI looks good! :)

Haven't reviewed completely or tested yet, but skimmed through it and had some feedback to share.

app/src/main/AndroidManifest.xml

nicolas-raoul · 2024-08-25T07:49:59Z

"What about I add a new progress bar using Jetpack"

That would be fantastic, thanks a lot. 🙂
Even a simple label x/y would be enough for now.

rohit9625 · 2024-08-25T13:37:31Z

"What about I add a new progress bar using Jetpack"

That would be fantastic, thanks a lot. 🙂 Even a simple label x/y would be enough for now.

The fragments were built in Java so I had to replace the circular progress bar with the material one(XML Views).

rohit9625 · 2024-08-25T14:26:04Z

Hey @nicolas-raoul, please try opening settings from the app. On my device, the app is crashing and maybe, this is the reason that testSetTotalUploadCount is failing.

nicolas-raoul · 2024-08-26T01:15:41Z

It seems like I am able to open settings (GSoC branch):

nicolas-raoul · 2024-08-26T03:15:40Z

I just tested this branch, it builds fine. 🙂

The 3 stats (for an empty account) all appear differently, is it expected? If difficult don't worry, priority is low so we can make this a different issue. 👍

rohit9625 · 2024-08-26T03:39:15Z

I just tested this branch, it builds fine. 🙂

The 3 stats (for an empty account) all appear differently, is it expected? If difficult don't worry, priority is low so we can make this a different issue. 👍

I didn't checked for an empty account. Checking if it is minor bug, then I'll fix it.

RitikaPahwa4444

Skimmed through the code again, had a few minor suggestions. Need some time to upgrade my Android Studio (the AGP version in this PR is not compatible with mine) but will hopefully test it soon. Thanks for refactoring the code as well 🙂

app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java

app/src/main/java/fr/free/nrw/commons/upload/worker/UploadWorker.kt

RitikaPahwa4444 · 2024-09-15T08:37:02Z

Any idea why the app is no longer showing notification for the foreground service or even the ongoing uploads? We currently display two of them, but on this branch I see none.

rohit9625 · 2024-09-15T08:51:22Z

Any idea why the app is no longer showing notification for the foreground service or even the ongoing uploads? We currently display two of them, but on this branch I see none.

No idea :(
I am looking into it. Can you please test the location problem again?

rohit9625 · 2024-09-15T09:12:39Z

Both notifications are showing when I am uploading new images. Make sure you have notification permission.

RitikaPahwa4444 · 2024-09-15T09:13:55Z

It didn't ask me for notification permission at all. Happened on a fresh installation. The app asks for it in the beginning itself on the main branch

rohit9625 · 2024-09-15T09:26:14Z

But, it asked me when I opened that newly implemented screen for managing uploads. Same behavior on the main branch. I think the app is asking for notification permission only when needed or after some time, not sure either.

RitikaPahwa4444 · 2024-09-15T09:29:11Z

I think the app is asking for notification permission only when needed or after some time, not sure either.

I've tried reinstalling the app to observe if that notification permission pops up, but I'm still waiting. It's been over 15 minutes already, I'm uploading pictures but not getting any notification or any dialog requesting access.

RitikaPahwa4444 · 2024-09-15T09:31:08Z

Custom selector is now working fine 🎉 The location info is not getting redacted and I can see the Commons logo on already uploaded images now

rohit9625 · 2024-09-15T09:33:13Z

Custom selector is now working fine 🎉 The location info is not getting redacted andi can see the Commons logo on already uploaded images now

What is the procedure to repeat that location reading process? Need to check if that is working fine on the main or not.

RitikaPahwa4444 · 2024-09-15T09:34:10Z

Closed and reopened the app and got this crash:

STACK_TRACE=com.bumptech.glide.load.engine.CallbackException: Unexpected exception thrown by non-Glide code
at com.bumptech.glide.load.engine.EngineJob.callCallbackOnResourceReady(EngineJob.java:161)
at com.bumptech.glide.load.engine.EngineJob$CallResourceReady.run(EngineJob.java:428)
at android.os.Handler.handleCallback(Handler.java:1000)
at android.os.Handler.dispatchMessage(Handler.java:104)
at android.os.Looper.loopOnce(Looper.java:242)
at android.os.Looper.loop(Looper.java:362)
at android.app.ActivityThread.main(ActivityThread.java:8393)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:992)
Caused by: java.lang.IllegalStateException: Fragment ExploreMapFragment{dd4f3da} (c3906b03-bb95-4c56-8865-cf6135e5da37) not attached to a context.
at androidx.fragment.app.Fragment.requireContext(Fragment.java:900)
at androidx.fragment.app.Fragment.getResources(Fragment.java:964)
at fr.free.nrw.commons.explore.map.ExploreMapFragment.addMarkerToMap(ExploreMapFragment.java:649)
at fr.free.nrw.commons.explore.map.ExploreMapFragment.addMarkersToMap(ExploreMapFragment.java:636)
at fr.free.nrw.commons.explore.map.ExploreMapPresenter.onNearbyBaseMarkerThumbsReady(ExploreMapPresenter.java:182)
at fr.free.nrw.commons.explore.map.ExploreMapController$1.onResourceReady(ExploreMapController.java:180)
at fr.free.nrw.commons.explore.map.ExploreMapController$1.onResourceReady(ExploreMapController.java:170)
at com.bumptech.glide.request.SingleRequest.onResourceReady(SingleRequest.java:639)
at com.bumptech.glide.request.SingleRequest.onResourceReady(SingleRequest.java:578)
at com.bumptech.glide.load.engine.EngineJob.callCallbackOnResourceReady(EngineJob.java:159)
... 9 more

Please don't worry if this is not related to your changes. Let's keep a track of this, though. I'll raise a separate issue if I face it frequently on the main branch as well.

rohit9625 · 2024-09-15T09:35:39Z

I think the app is asking for notification permission only when needed or after some time, not sure either.

I've tried reinstalling the app to observe if that notification permission pops up, but I'm still waiting. It's been over 15 minutes already, I'm uploading pictures but not getting any notification or any dialog requesting access.

Did you try the same on the main. Because even on main branch the app is not requesting the notification permission. I tried it myself just few minutes ago.

rohit9625 · 2024-09-15T09:39:55Z

Please don't worry if this is not related to your changes. Let's keep a track of this, though. I'll raise a separate issue if I face it frequently on the main branch as well.

Okay 👍, that'll be nice :)
Even I also noticed a behavior related to pausing uploads. I'll create a new issue for that.

RitikaPahwa4444 · 2024-09-15T09:44:15Z

Because even on main branch the app is not requesting the notification permission

But you still see the notifications, right? 🤔

rohit9625 · 2024-09-15T09:50:40Z

But you still see the notifications, right? 🤔

No, I tried uploading a new picture from the main branch version but no notifications showed up. So, I opened the screen where we can manage uploads and then the app asked me for notification permission. After that, the uploading seemed to restart (Not sure) and both notifications were showing.

RitikaPahwa4444 · 2024-09-15T09:52:10Z

I've opened the pending uploads screen several times on this branch but didn't get that permission popup. Testing on main now.

rohit9625 · 2024-09-15T09:56:20Z

I've opened the pending uploads screen several times on this branch but didn't get that permission popup. Testing on main now.

Just to be sure. Did you open that screen just after starting a new upload? Because ongoing uploads are the reason to ask for notification permission, not opening that screen only. However, it seems another issue and out of the scope of this PR. What do you think?

RitikaPahwa4444 · 2024-09-15T10:08:45Z

Did you open that screen just after starting a new upload?

Yes.

However, it seems another issue and out of the scope of this PR

Bumping versions does break things. And if notifications for foreground services have disappeared without the user dismissing them, we cannot rest assured that they would work as intended without tesing completely. I'm sorry if you found this unrelated - I'm just testing and sharing what is different on this branch.

rohit9625 · 2024-09-15T10:12:46Z

Custom selector is now working fine 🎉 The location info is not getting redacted and I can see the Commons logo on already uploaded images now

I tried uploading a picture that has a location in its EXIF and the app can read the location info from EXIF of the image after the recent commit. However, I am getting these errors on the logs on this PR and main as well.

rohit9625 · 2024-09-15T10:19:32Z

However, it seems another issue and out of the scope of this PR

Bumping versions does break things. And if notifications for foreground services have disappeared without the user dismissing them, we cannot rest assured that they would work as intended without testing completely. I'm sorry if you found this unrelated - I'm just testing and sharing what is different on this branch.

You are right, bumping to a new SDK definitely can break other parts. However, it's working on API 34 but the behavior could be different on other versions. What API level you are testing on?
Thank you once again for your insights :)

RitikaPahwa4444 · 2024-09-15T10:39:50Z

I'm testing on Android 14 (API level 34).

rohit9625 · 2024-09-15T10:41:37Z

I'm testing on Android 14 (API level 34).

Now, what am I supposed to do? I am so confused :(

rohit9625 · 2024-09-16T06:10:53Z

@RitikaPahwa4444, I can make the app request Notification Permission at the startup to solve the problem. However, users can decline the permission. Would you happen to have any reference to the current implementation of requesting notification permission?

RitikaPahwa4444 · 2024-09-16T14:35:14Z

Hey @rohit9625, I would need some time to check on the notifications issue along with the issue mentioned in this comment. Until then, please don't consider them as a blocker. The location issue seems sorted for me at least, but I'll share if I find any errors in my logs. Keeping the notifications issue aside, uploads seem to work fine for me while I'm using the app.

rohit9625 · 2024-09-16T14:43:53Z

Thank you @RitikaPahwa4444 & @nicolas-raoul for your time on this PR :)
Now, if everything seems sorted then what about merging this PR?

nicolas-raoul · 2024-09-17T14:58:38Z

I believe Ritika wrote that there is no blocker, so I will merge this.
I uploaded hundreds of pictures with this branch and it worked fine. Unit tests passing too.
Thanks a lot @rohit9625, this was high priority because it was blocking further releases.

rohit9625 added 9 commits August 23, 2024 15:09

change the overridden method signature as per API 34

9db025b

add version check condition to compare with API 23 before adding flag

b5b40ac

refactor: add final keywords, fix typo, and remove redundant spaces

c0a28d0

For optimized code only

upgrade: migrate to SDK 34 and upgrade APG

1c9609e

Additionally, add Jetpack Compose to the project

AndroidManifest: add new permission for API 34

81cc53a

DescriptionActivity should not be exposed

refactor: permission should not be check on onCreate for some cases

ef18897

add method to get correct storage permission and check partial access

bf68861

Additionally, add final keywords to reduce compiler warnings

refactor: prevent app from crashing for SDKs >= 34

8ec9118

add new UI component to allows user to manage partially access photos

9c683aa

Implement using composeView

rohit9625 added 9 commits August 25, 2024 09:49

change the overridden method signature as per API 34

ba573ed

add version check condition to compare with API 23 before adding flag

21b2184

refactor: add final keywords, fix typo, and remove redundant spaces

ee55b59

For optimized code only

upgrade: migrate to SDK 34 and upgrade APG

8471928

Additionally, add Jetpack Compose to the project

AndroidManifest: add new permission for API 34

1995c0a

DescriptionActivity should not be exposed

refactor: permission should not be check on onCreate for some cases

349b8ef

add method to get correct storage permission and check partial access

f8d164b

Additionally, add final keywords to reduce compiler warnings

refactor: prevent app from crashing for SDKs >= 34

ad1074a

add new UI component to allows user to manage partially access photos

bc621a0

Implement using composeView

rohit9625 force-pushed the upgrade branch from 9c683aa to bc621a0 Compare August 25, 2024 04:19

Merge branch 'upgrade' of https://github.com/rohit9625/apps-android-c…

c2c6dc2

…ommons into upgrade

RitikaPahwa4444 requested changes Aug 25, 2024

View reviewed changes

app/src/main/AndroidManifest.xml Show resolved Hide resolved

replace deprecated circular progress bar with material progress bar

2223e6d

RitikaPahwa4444 requested changes Sep 15, 2024

View reviewed changes

app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java Show resolved Hide resolved

app/src/main/java/fr/free/nrw/commons/upload/worker/UploadWorker.kt Outdated Show resolved Hide resolved

add permission to access media location

543840c

nicolas-raoul approved these changes Sep 17, 2024

View reviewed changes

rohit9625 requested a review from RitikaPahwa4444 September 17, 2024 14:49

nicolas-raoul merged commit 3e915f9 into commons-app:main Sep 17, 2024
1 check passed

rohit9625 deleted the upgrade branch September 20, 2024 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to SDK 34 #5790

Upgrade to SDK 34 #5790

rohit9625 commented Aug 24, 2024 •

edited

Loading

nicolas-raoul commented Aug 25, 2024 •

edited

Loading

rohit9625 commented Aug 25, 2024

RitikaPahwa4444 left a comment

nicolas-raoul commented Aug 25, 2024

rohit9625 commented Aug 25, 2024

rohit9625 commented Aug 25, 2024

nicolas-raoul commented Aug 26, 2024

nicolas-raoul commented Aug 26, 2024

rohit9625 commented Aug 26, 2024 •

edited

Loading

RitikaPahwa4444 left a comment

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024 •

edited

Loading

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024 •

edited

Loading

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024 •

edited

Loading

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

rohit9625 commented Sep 16, 2024

RitikaPahwa4444 commented Sep 16, 2024

rohit9625 commented Sep 16, 2024 •

edited

Loading

nicolas-raoul commented Sep 17, 2024

Upgrade to SDK 34 #5790

Upgrade to SDK 34 #5790

Conversation

rohit9625 commented Aug 24, 2024 • edited Loading

nicolas-raoul commented Aug 25, 2024 • edited Loading

rohit9625 commented Aug 25, 2024

RitikaPahwa4444 left a comment

Choose a reason for hiding this comment

nicolas-raoul commented Aug 25, 2024

rohit9625 commented Aug 25, 2024

rohit9625 commented Aug 25, 2024

nicolas-raoul commented Aug 26, 2024

nicolas-raoul commented Aug 26, 2024

rohit9625 commented Aug 26, 2024 • edited Loading

RitikaPahwa4444 left a comment

Choose a reason for hiding this comment

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024 • edited Loading

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024 • edited Loading

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024 • edited Loading

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

RitikaPahwa4444 commented Sep 15, 2024

rohit9625 commented Sep 15, 2024

rohit9625 commented Sep 16, 2024

RitikaPahwa4444 commented Sep 16, 2024

rohit9625 commented Sep 16, 2024 • edited Loading

nicolas-raoul commented Sep 17, 2024

rohit9625 commented Aug 24, 2024 •

edited

Loading

nicolas-raoul commented Aug 25, 2024 •

edited

Loading

rohit9625 commented Aug 26, 2024 •

edited

Loading

RitikaPahwa4444 commented Sep 15, 2024 •

edited

Loading

RitikaPahwa4444 commented Sep 15, 2024 •

edited

Loading

rohit9625 commented Sep 15, 2024 •

edited

Loading

rohit9625 commented Sep 16, 2024 •

edited

Loading