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.

Sign up for GitHub

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

[TECHNICAL] Technical improvements for user quota #4525

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[TECHNICAL] Technical improvements for user quota #4525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7324483
feat: added a new use case to fetch the user quota as Flow (`GetStore…
joragua Dec 11, 2024
5525e42
test: added some necessary tests from `OCLocalUserDataSourceTest` and…
joragua Dec 11, 2024
8d5c35f
chore: added calens file
joragua Dec 11, 2024
3a3a4e2
docs: calens changelog updated
joragua Dec 11, 2024
6ab970d
refactor: added identation in `DrawerViewModelTest`
joragua Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ ownCloud admins and users.
* Enhancement - Added text labels for BottomNavigationView: [#4484](https://github.com/owncloud/android/issues/4484)
* Enhancement - OCIS Light Users: [#4490](https://github.com/owncloud/android/issues/4490)
* Enhancement - Enforce OIDC auth flow via branding: [#4500](https://github.com/owncloud/android/issues/4500)
* Enhancement - Technical improvements for user quota: [#4521](https://github.com/owncloud/android/issues/4521)

## Details

Expand Down Expand Up @@ -114,6 +115,14 @@ ownCloud admins and users.
https://github.com/owncloud/android/issues/4500
https://github.com/owncloud/android/pull/4516

* Enhancement - Technical improvements for user quota: [#4521](https://github.com/owncloud/android/issues/4521)

A new use case has been added to fetch the user quota as a flow. Also, all
unnecessary calls from DrawerActivity have been removed.

https://github.com/owncloud/android/issues/4521
https://github.com/owncloud/android/pull/4525

# Changelog for ownCloud Android Client [4.4.1] (2024-10-30)

The following sections list the changes in ownCloud Android Client 4.4.1 relevant to
Expand Down
7 changes: 7 additions & 0 deletions changelog/unreleased/4525
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Enhancement: Technical improvements for user quota

A new use case has been added to fetch the user quota as a flow.
Also, all unnecessary calls from DrawerActivity have been removed.

https://github.com/owncloud/android/issues/4521
https://github.com/owncloud/android/pull/4525
3 changes: 3 additions & 0 deletions owncloudApp/src/main/java/com/owncloud/android/dependecyinjection/UseCaseModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* @author Abel García de Prada
* @author Juan Carlos Garrote Gascón
* @author Aitor Ballesteros Pavón
* @author Jorge Aguado Recio
*
* Copyright (C) 2024 ownCloud GmbH.
*
Expand Down Expand Up @@ -101,6 +102,7 @@ import com.owncloud.android.domain.transfers.usecases.GetAllTransfersAsStreamUse
import com.owncloud.android.domain.transfers.usecases.GetAllTransfersUseCase
import com.owncloud.android.domain.transfers.usecases.UpdatePendingUploadsPathUseCase
import com.owncloud.android.domain.user.usecases.GetStoredQuotaUseCase
import com.owncloud.android.domain.user.usecases.GetStoredQuotaAsStreamUseCase
import com.owncloud.android.domain.user.usecases.GetUserAvatarAsyncUseCase
import com.owncloud.android.domain.user.usecases.GetUserInfoAsyncUseCase
import com.owncloud.android.domain.user.usecases.GetUserQuotasUseCase
Expand Down Expand Up @@ -252,6 +254,7 @@ val useCaseModule = module {
factoryOf(::UploadFilesFromSystemUseCase)

// User
factoryOf(::GetStoredQuotaAsStreamUseCase)
factoryOf(::GetStoredQuotaUseCase)
factoryOf(::GetUserAvatarAsyncUseCase)
factoryOf(::GetUserInfoAsyncUseCase)
Expand Down
32 changes: 5 additions & 27 deletions owncloudApp/src/main/java/com/owncloud/android/presentation/common/DrawerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,65 +25,43 @@ package com.owncloud.android.presentation.common

import android.accounts.Account
import android.content.Context
import androidx.lifecycle.LiveData
import androidx.lifecycle.MediatorLiveData
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.owncloud.android.R
import com.owncloud.android.data.providers.LocalStorageProvider
import com.owncloud.android.domain.user.model.UserQuota
import com.owncloud.android.domain.user.usecases.GetStoredQuotaUseCase
import com.owncloud.android.domain.user.usecases.GetStoredQuotaAsStreamUseCase
import com.owncloud.android.domain.user.usecases.GetUserQuotasUseCase
import com.owncloud.android.domain.utils.Event
import com.owncloud.android.extensions.ViewModelExt.runUseCaseWithResult
import com.owncloud.android.presentation.authentication.AccountUtils
import com.owncloud.android.providers.ContextProvider
import com.owncloud.android.providers.CoroutinesDispatcherProvider
import com.owncloud.android.usecases.accounts.RemoveAccountUseCase
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.launch
import timber.log.Timber

class DrawerViewModel(
private val getStoredQuotaUseCase: GetStoredQuotaUseCase,
getStoredQuotaAsStreamUseCase: GetStoredQuotaAsStreamUseCase,
private val removeAccountUseCase: RemoveAccountUseCase,
private val getUserQuotasUseCase: GetUserQuotasUseCase,
private val localStorageProvider: LocalStorageProvider,
private val coroutinesDispatcherProvider: CoroutinesDispatcherProvider,
private val contextProvider: ContextProvider,
accountName: String,
) : ViewModel() {

private val _userQuota = MediatorLiveData<Event<UIResult<UserQuota?>>>()
val userQuota: LiveData<Event<UIResult<UserQuota?>>> = _userQuota

fun getStoredQuota(
accountName: String
) = runUseCaseWithResult(
coroutineDispatcher = coroutinesDispatcherProvider.io,
requiresConnection = false,
showLoading = true,
liveData = _userQuota,
useCase = getStoredQuotaUseCase,
useCaseParams = GetStoredQuotaUseCase.Params(accountName = accountName)
)
val userQuota: Flow<UserQuota> = getStoredQuotaAsStreamUseCase(GetStoredQuotaAsStreamUseCase.Params(accountName))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, not sure if I like this... I think it would be better being encapsulated with a private variable. Otherwise everytime we access userQuota, the use case might be executed, and that's not what we want. Check for example SettingsPictureUploadsViewModel to see how this is done with the pictureUploads variable 👍


fun getAccounts(context: Context): List<Account> {
return AccountUtils.getAccounts(context).asList()
}

fun getCurrentAccount(context: Context): Account? {
return AccountUtils.getCurrentOwnCloudAccount(context)
}

fun getUsernameOfAccount(accountName: String): String {
return AccountUtils.getUsernameOfAccount(accountName)
}

fun getFeedbackMail() = contextProvider.getString(R.string.mail_feedback)

fun setCurrentAccount(context: Context, accountName: String): Boolean {
return AccountUtils.setCurrentOwnCloudAccount(context, accountName)
}

fun removeAccount(context: Context) {
viewModelScope.launch(coroutinesDispatcherProvider.io) {
val loggedAccounts = AccountUtils.getAccounts(context)
Expand Down
210 changes: 102 additions & 108 deletions owncloudApp/src/main/java/com/owncloud/android/ui/activity/DrawerActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import com.owncloud.android.domain.capabilities.model.OCCapability
import com.owncloud.android.domain.files.model.FileListOption
import com.owncloud.android.domain.user.model.UserQuotaState
import com.owncloud.android.domain.utils.Event
import com.owncloud.android.extensions.collectLatestLifecycleFlow
import com.owncloud.android.extensions.goToUrl
import com.owncloud.android.extensions.openPrivacyPolicy
import com.owncloud.android.extensions.sendEmailOrOpenFeedbackDialogAction
Expand All @@ -82,7 +83,11 @@ import timber.log.Timber
*/
abstract class DrawerActivity : ToolbarActivity() {

private val drawerViewModel by viewModel<DrawerViewModel>()
private val drawerViewModel by viewModel<DrawerViewModel> {
parametersOf(
account?.name
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if account is null? Did you check where this value is set in BaseActivity?

)
}
private val capabilitiesViewModel by viewModel<CapabilityViewModel> {
parametersOf(
account?.name
Expand Down Expand Up @@ -287,7 +292,6 @@ abstract class DrawerActivity : ToolbarActivity() {
open fun openDrawer() {
getDrawerLayout()?.openDrawer(GravityCompat.START)
findViewById<View>(R.id.nav_view).requestFocus()
drawerViewModel.getStoredQuota(account.name)
}

/**
Expand All @@ -305,115 +309,105 @@ abstract class DrawerActivity : ToolbarActivity() {
*/
private fun updateQuota() {
Timber.d("Update Quota")
val account = drawerViewModel.getCurrentAccount(this) ?: return
drawerViewModel.getStoredQuota(account.name)
drawerViewModel.userQuota.observe(this) { event ->
when (val uiResult = event.peekContent()) {
is UIResult.Success -> {
uiResult.data?.let { userQuota ->
when {
userQuota.available == -4L -> { // Light users (oCIS)
getAccountQuotaText()?.text = getString(R.string.drawer_unavailable_used_storage)
getAccountQuotaBar()?.isVisible = false
getAccountQuotaStatusText()?.isVisible = false
}

userQuota.available < 0 -> { // Pending, unknown or unlimited free storage
getAccountQuotaBar()?.apply {
isVisible = true
progress = 0
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.color_accent))
}
getAccountQuotaText()?.text = String.format(
getString(R.string.drawer_unavailable_free_storage),
DisplayUtils.bytesToHumanReadable(userQuota.used, this, true)
)
getAccountQuotaStatusText()?.visibility = View.GONE
}

userQuota.available == 0L -> { // Exceeded storage. The value is over 100%.
getAccountQuotaBar()?.apply {
isVisible = true
progress = 100
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.quota_exceeded))
}

if (userQuota.state == UserQuotaState.EXCEEDED) { // oCIS
getAccountQuotaText()?.apply {
text = String.format(
getString(R.string.drawer_quota),
DisplayUtils.bytesToHumanReadable(userQuota.used, context, true),
DisplayUtils.bytesToHumanReadable(userQuota.getTotal(), context, true),
userQuota.getRelative()
)
}
getAccountQuotaStatusText()?.apply {
visibility = View.VISIBLE
text = getString(R.string.drawer_exceeded_quota)
}
} else { // oC10
getAccountQuotaText()?.text = getString(R.string.drawer_exceeded_quota)
getAccountQuotaStatusText()?.visibility = View.GONE
}
}

else -> { // Limited storage. Value under 100%
if (userQuota.state == UserQuotaState.NEARING) { // Nearing storage. Value between 75% and 90%
getAccountQuotaBar()?.apply {
isVisible = true
progress = userQuota.getRelative().toInt()
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.color_accent))
}
getAccountQuotaText()?.apply {
text = String.format(
getString(R.string.drawer_quota),
DisplayUtils.bytesToHumanReadable(userQuota.used, context, true),
DisplayUtils.bytesToHumanReadable(userQuota.getTotal(), context, true),
userQuota.getRelative()
)
}
getAccountQuotaStatusText()?.apply {
visibility = View.VISIBLE
text = getString(R.string.drawer_nearing_quota)
}
} else if (userQuota.state == UserQuotaState.CRITICAL || userQuota.state == UserQuotaState.EXCEEDED) { // Critical storage. Value over 90%
getAccountQuotaBar()?.apply {
isVisible = true
progress = userQuota.getRelative().toInt()
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.quota_exceeded))
}
getAccountQuotaText()?.apply {
text = String.format(
getString(R.string.drawer_quota),
DisplayUtils.bytesToHumanReadable(userQuota.used, context, true),
DisplayUtils.bytesToHumanReadable(userQuota.getTotal(), context, true),
userQuota.getRelative()
)
}
getAccountQuotaStatusText()?.apply {
visibility = View.VISIBLE
text = getString(R.string.drawer_critical_quota)
}
} else { // Normal storage. Value under 75%
getAccountQuotaBar()?.apply {
progress = userQuota.getRelative().toInt()
isVisible = true
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.color_accent))
}
getAccountQuotaText()?.text = String.format(
getString(R.string.drawer_quota),
DisplayUtils.bytesToHumanReadable(userQuota.used, this, true),
DisplayUtils.bytesToHumanReadable(userQuota.getTotal(), this, true),
userQuota.getRelative()
)
getAccountQuotaStatusText()?.visibility = View.GONE
}
}
collectLatestLifecycleFlow(drawerViewModel.userQuota) { userQuota ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

I did some changes in all these lines in the Detekt branch, I think just for the MaxLineLength rule. Try to keep them in the new place for these lines so that we don't duplicate work

when {
userQuota.available == -4L -> { // Light users (oCIS)
getAccountQuotaText()?.text = getString(R.string.drawer_unavailable_used_storage)
getAccountQuotaBar()?.isVisible = false
getAccountQuotaStatusText()?.isVisible = false
}

userQuota.available < 0 -> { // Pending, unknown or unlimited free storage
getAccountQuotaBar()?.apply {
isVisible = true
progress = 0
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.color_accent))
}
getAccountQuotaText()?.text = String.format(
getString(R.string.drawer_unavailable_free_storage),
DisplayUtils.bytesToHumanReadable(userQuota.used, this, true)
)
getAccountQuotaStatusText()?.visibility = View.GONE
}

userQuota.available == 0L -> { // Exceeded storage. The value is over 100%.
getAccountQuotaBar()?.apply {
isVisible = true
progress = 100
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.quota_exceeded))
}

if (userQuota.state == UserQuotaState.EXCEEDED) { // oCIS
getAccountQuotaText()?.apply {
text = String.format(
getString(R.string.drawer_quota),
DisplayUtils.bytesToHumanReadable(userQuota.used, context, true),
DisplayUtils.bytesToHumanReadable(userQuota.getTotal(), context, true),
userQuota.getRelative()
)
}
getAccountQuotaStatusText()?.apply {
visibility = View.VISIBLE
text = getString(R.string.drawer_exceeded_quota)
}
} else { // oC10
getAccountQuotaText()?.text = getString(R.string.drawer_exceeded_quota)
getAccountQuotaStatusText()?.visibility = View.GONE
}
}

else -> { // Limited storage. Value under 100%
if (userQuota.state == UserQuotaState.NEARING) { // Nearing storage. Value between 75% and 90%
getAccountQuotaBar()?.apply {
isVisible = true
progress = userQuota.getRelative().toInt()
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.color_accent))
}
getAccountQuotaText()?.apply {
text = String.format(
getString(R.string.drawer_quota),
DisplayUtils.bytesToHumanReadable(userQuota.used, context, true),
DisplayUtils.bytesToHumanReadable(userQuota.getTotal(), context, true),
userQuota.getRelative()
)
}
getAccountQuotaStatusText()?.apply {
visibility = View.VISIBLE
text = getString(R.string.drawer_nearing_quota)
}
} else if (userQuota.state == UserQuotaState.CRITICAL || userQuota.state == UserQuotaState.EXCEEDED) { // Critical storage. Value over 90%
getAccountQuotaBar()?.apply {
isVisible = true
progress = userQuota.getRelative().toInt()
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.quota_exceeded))
}
getAccountQuotaText()?.apply {
text = String.format(
getString(R.string.drawer_quota),
DisplayUtils.bytesToHumanReadable(userQuota.used, context, true),
DisplayUtils.bytesToHumanReadable(userQuota.getTotal(), context, true),
userQuota.getRelative()
)
}
getAccountQuotaStatusText()?.apply {
visibility = View.VISIBLE
text = getString(R.string.drawer_critical_quota)
}
} else { // Normal storage. Value under 75%
getAccountQuotaBar()?.apply {
progress = userQuota.getRelative().toInt()
isVisible = true
progressTintList = ColorStateList.valueOf(resources.getColor(R.color.color_accent))
}
getAccountQuotaText()?.text = String.format(
getString(R.string.drawer_quota),
DisplayUtils.bytesToHumanReadable(userQuota.used, this, true),
DisplayUtils.bytesToHumanReadable(userQuota.getTotal(), this, true),
userQuota.getRelative()
)
getAccountQuotaStatusText()?.visibility = View.GONE
}
}
is UIResult.Loading -> getAccountQuotaText()?.text = getString(R.string.drawer_loading_quota)
is UIResult.Error -> getAccountQuotaText()?.text = getString(R.string.drawer_unavailable_used_storage)
Comment on lines -415 to -416
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're losing these 2 cases!! We shouldn't, they are so necessary

}
}
}
Expand Down
Loading
Loading