[FEATURE] Get Personal spaces quota from GraphAPI

joragua commented 1 month ago

[x] Add changelog files for the fixed issues in folder changelog/unreleased. More info here
[x] Add feature to Release Notes in ReleaseNotesViewModel.kt creating a new ReleaseNote() with String resources (if required)

QA

Test plan: https://github.com/owncloud/QA/blob/master/Mobile/Android/Executions/Release_4.3/Quota.md

Reports:

[x] (1) Quota updated https://github.com/owncloud/android/pull/4401#issuecomment-2112894204 [FIXED]
[x] (2) "No restriction" value shows a limit https://github.com/owncloud/android/pull/4401#issuecomment-2122914804 [FIXED]
[X] (3) Improvements https://github.com/owncloud/android/pull/4401#issuecomment-2122995186 [WONT FIX]
[x] (4) Crash on fresh install https://github.com/owncloud/android/pull/4401#issuecomment-2127026972 [FIXED]

jesmrec commented 1 month ago

(1) [FIXED]

How or when is the quota checked? i mean, after adding an account, qouta data is retrieved and displayed. But, if quota changes later, is it rechecked and updated? i wasn't able to.

jesmrec commented 1 month ago

Let's do a quick look over the current behaviour

current branch	current `master`	`v4.2.1`
oC10	Correct after login. After a couple of reopenings, it is refreshed ✅	Correct after login. After a couple of reopenings, it is refreshed ✅	Correct after login. After a couple of reopenings, it is refreshed ✅
oCIS	No updates beyond the login ❌	Correct after login. After a couple of reopenings, it is refreshed ✅	Correct after login. If quota changes, it is refreshed but not immediately. Need some reopens (don't know why) ✅

that means, i see changes in the quota in both master and 4.2.1 for both backends. The new current scenario is where i don't see updates.

jesmrec commented 1 month ago

(2) [FIXED]

In oCIS account, set quota to "No restriction"
Refresh quota in Android app

Current:

This is what GraphAPI returns:

            "quota": {
                "remaining": 26283307008,
                "state": "normal",
                "total": 0,
                "used": 382726391
            },

and this is displayed in app:

Screenshot 2024-05-21 at 17 33 29

Expected:

If "total" == 0 , there is no limit, so the amount to display would be only the "used" (365MB in the example)

Pixel 2, Android 11 484062c4c

jesmrec commented 1 month ago

(3)[IMPROVEMENT]

In case the quota is exceeded, we are showing a message No storage usage information available

In this case, GraphAPI returns something like:

            "quota": {
                "remaining": 0,
                "state": "exceeded",
                "total": 1000000000,
                "used": 1423007244
            },

so, we have information about it (state == exceeded)... options that come to my mind:

Showing the same information as the non-exceeded quota with an extra message "Quota exceeded" or similar. Using red color (i think that text is not brandable)
Showing only "Quota exceeded"

As extra ball.. apart of "normal" and "exceeded", there is another value for the status that is "nearing" when the quota is close to the limit.

Also, adding the request after adding/deleting.

For sure, these are improvements and could be addressed to another issue.

joragua commented 1 month ago

FIXES

About (1): A refresh for spaces has been added when FileDisplayActivity is created.

About (2) : At first, the value of total quota is checked. If it's 0 then available value is set to -3 (the same as oC10). If that value it's no 0 then available value is the remaining one from GraphAPI.

jesmrec commented 1 month ago

(1) and (2) fixed.

will create a new issue for (3)

jesmrec commented 1 month ago

(4)

App crashes with frecuency after fresh install.. not always (unfortunately). I was diving a little bit, and i fount the following stacktrace

...
 Caused by: java.lang.NullPointerException: Attempt to read from field 'java.lang.String android.accounts.Account.name' on a null object reference
                    at com.owncloud.android.ui.activity.FileDisplayActivity$spacesListViewModel$2.invoke(FileDisplayActivity.kt:180)
                    at com.owncloud.android.ui.activity.FileDisplayActivity$spacesListViewModel$2.invoke(FileDisplayActivity.kt:179)
                    at org.koin.core.scope.Scope.resolveInstance(Scope.kt:223)
                    at org.koin.core.scope.Scope.get(Scope.kt:210)
...

pointing to FileDisplayActivity lines 179 and 180, that just were added in he current PR:

 private val spacesListViewModel: SpacesListViewModel by viewModel {
        parametersOf(account.name, false)
    }

that comes to my mind together with https://github.com/owncloud/android/pull/4408, where the null account is protected.

joragua commented 1 month ago

(4) can be fixed refreshing all spaces when the account is set (onAccountSet) and not in onCreate. This solution would be:

private val spacesListViewModel: SpacesListViewModel by viewModel {
            parametersOf(account.name, false)
}

spacesListViewModel.refreshSpacesFromServer()

jesmrec commented 1 month ago

(4) fixed, and PR approved

Merge blocked till 4.2.2 is out

owncloud / android