Fix #5213 app is excluded from recent/current apps

[JorisBodin]

After talking with @tobiasKaminsky, we've decided to remove this finish()
Because "onResume" is called after requestPermissionsResult for PERMISSIONS_WRITE_EXTERNAL_STORAGE

#5213

[nextcloud-android-bot]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12311.apk



To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

[nextcloud-android-bot]

Codacy

340

Lint

Type	master	PR
Warnings	76	76
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	71
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	107
Security Warnings	44
Dodgy code Warnings	138
Total	413

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	26
Correctness Warnings	71
Internationalization Warnings	13
Malicious code vulnerability Warnings	5
Multithreaded correctness Warnings	9
Performance Warnings	107
Security Warnings	44
Dodgy code Warnings	138
Total	413

[codecov]

Codecov Report

Merging #5218 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #5218      +/-   ##
============================================
+ Coverage     17.69%   17.71%   +0.02%     
  Complexity        3        3              
============================================
  Files           392      392              
  Lines         33140    33138       -2     
  Branches       4656     4655       -1     
============================================
+ Hits           5864     5872       +8     
+ Misses        26319    26308      -11     
- Partials        957      958       +1

Impacted Files	Coverage Δ	Complexity Δ
...com/owncloud/android/ui/activity/BaseActivity.java	38.46% <ø> (+0.82%)	0 <0> (ø)	⬇️
.../third_parties/daveKoeller/AlphanumComparator.java	80.95% <0%> (-1.2%)	0% <0%> (ø)
...ncloud/android/ui/fragment/OCFileListFragment.java	24.9% <0%> (-0.4%)	0% <0%> (ø)
...oud/android/ui/activity/SyncedFoldersActivity.java	26.81% <0%> (+0.5%)	0% <0%> (ø)	⬇️
...loud/android/datamodel/ThumbnailsCacheManager.java	32.84% <0%> (+0.54%)	0% <0%> (ø)	⬇️
.../java/com/owncloud/android/utils/DisplayUtils.java	25.73% <0%> (+0.84%)	0% <0%> (ø)	⬇️
...om/owncloud/android/utils/FileSortOrderByName.java	17.14% <0%> (+2.85%)	0% <0%> (ø)	⬇️
...ain/java/com/owncloud/android/ui/TextDrawable.java	81.48% <0%> (+7.4%)	0% <0%> (ø)	⬇️
...d/android/operations/GetCapabilitiesOperation.java	87.5% <0%> (+12.5%)	0% <0%> (ø)	⬇️
...m/nextcloud/client/account/UserAccountManager.java	33.33% <0%> (+33.33%)	0% <0%> (ø)	⬇️

[ezaquarii]

I tested account creation and login and I see no problem described in #5031 (comment). This is good, but it could as well be a race condition.

Please test the following scenarios:

1. login
2. adding new account

In both cases application should not display empty file list.

[mario]

This is actually how it is for every permission request - the thing is that when you request permissions via requestPermissions the activity is paused. After answering, onResume gets called (again).

[mario]

So, to summarize: I'd suggest setting up all the views and everything and then ask for permissions.

[JorisBodin]

I tested login and adding new account. I have note empty files.

With finish()

Wihout finish()

[JorisBodin]

I encountered the problem without the finish. I had the request for authorization to write toi storage file on the login activity. So the activity that lists the files is launched while we are on that of login. Here is the problem. I'm watching this next week.

[ezaquarii]

That doesn't seem right given that:

1. this code path is hit only when there is no user
2. without user you can't instantiate storage manager to access files

[tobiasKaminsky]

Hm.
This permission is not needed, if you

• have new installation
• login for very first time
• browse files
• download file

It is only needed, if you want to upload a file from external storage, eg. external sdcard.
So we maybe should delay this permission question to the point we open up "+" and select "upload files"?

[JorisBodin]

That doesn't seem right given that:

1. this code path is hit only when there is no user
2. without user you can't instantiate storage manager to access files

Just to let you know it can happen when the app is launched.

@tobiasKaminsky @ezaquarii
This finish prevents the execution of FileDisplayActivity. Otherwise we have requestWriteExternalStoreagePermission, which is called line 313.

And that's why the file list is not refreshed after the first login.

[JorisBodin]

@ezaquarii
The problem is explained in more detail here: #4993

I think that adding this finish is not necessary (especially because it generates a bug). It is better to fix the problem at source, as described in this issue #4993

[tobiasKaminsky]

@ezaquarii this bug prevents user from switching during login to another app, e.g. for generating one-time password.

As the only drawback right now might be that we get an empty list on first start, I am fine with this.

@JorisBodin can you then remove the accountHandling boolean es we this then do not need it.

How to proceed, we should discuss in #4993

[JorisBodin]

@tobiasKaminsky if remove enableAccountHandling in baseActivity, accountManager.getCurrentAccount() is called.
So we go directly to the login view.

But it will have to be removed during the big correction of #4993

[tobiasKaminsky]

ah true.
Then let us keep it for now and do it right with #4993.

[JorisBodin]

@tobiasKaminsky So, do we merge it or do we close this MR?

[tobiasKaminsky]

/backport to stable-3.10

[backportbot-nextcloud]

backport to stable-3.10 in #5324