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

[FEATURE] Get Personal spaces quota from GraphAPI #4401

Merged
merged 9 commits into from
May 31, 2024

Conversation

joragua
Copy link
Collaborator

@joragua joragua commented May 9, 2024

Related Issues

App: #3874

  • Add changelog files for the fixed issues in folder changelog/unreleased. More info here
  • 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:

@joragua joragua self-assigned this May 9, 2024
@joragua joragua linked an issue May 9, 2024 that may be closed by this pull request
10 tasks
@joragua joragua force-pushed the feature/personal_space_quota branch 4 times, most recently from 2672d16 to b365045 Compare May 9, 2024 09:57
@joragua joragua marked this pull request as ready for review May 9, 2024 10:03
@joragua joragua requested a review from JuancaG05 May 9, 2024 10:04
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions and questions here @joragua 👍

@joragua joragua requested a review from JuancaG05 May 10, 2024 10:15
@joragua joragua force-pushed the feature/personal_space_quota branch from 8c49ad5 to 09dde3c Compare May 10, 2024 10:17
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the reply to one of the comments of the previous CR round and we're done 😉 @joragua

@joragua joragua requested a review from JuancaG05 May 15, 2024 10:32
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! 💯

@jesmrec
Copy link
Collaborator

jesmrec commented May 15, 2024

(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
Copy link
Collaborator

jesmrec commented May 16, 2024

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.

@joragua joragua force-pushed the feature/personal_space_quota branch from 79ab30c to 484062c Compare May 21, 2024 11:11
@jesmrec
Copy link
Collaborator

jesmrec commented May 21, 2024

(2) [FIXED]

  1. In oCIS account, set quota to "No restriction"
  2. 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
Copy link
Collaborator

jesmrec commented May 21, 2024

(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
Copy link
Collaborator Author

joragua commented May 22, 2024

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.

@joragua joragua requested a review from JuancaG05 May 22, 2024 09:10
@jesmrec
Copy link
Collaborator

jesmrec commented May 23, 2024

(1) and (2) fixed.

will create a new issue for (3)

@jesmrec
Copy link
Collaborator

jesmrec commented May 23, 2024

(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 #4408, where the null account is protected.

@joragua
Copy link
Collaborator Author

joragua commented May 24, 2024

(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
Copy link
Collaborator

jesmrec commented May 24, 2024

(4) fixed, and PR approved

Merge blocked till 4.2.2 is out

@JuancaG05 JuancaG05 force-pushed the feature/personal_space_quota branch from ad83e29 to 52c04c5 Compare May 31, 2024 12:13
@JuancaG05 JuancaG05 merged commit 9f6d14d into master May 31, 2024
5 checks passed
@JuancaG05 JuancaG05 deleted the feature/personal_space_quota branch May 31, 2024 12:40
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.

[SPACES] Get Personal spaces quota from GraphAPI
3 participants