From 89a818aeaa0f80744bbfaf4db25e66b40d7ad8a2 Mon Sep 17 00:00:00 2001 From: David Perez Date: Wed, 17 Jan 2024 22:58:13 -0600 Subject: [PATCH] BIT-493: Add full file send support (#651) --- .../feature/send/addsend/AddSendContent.kt | 99 +++++++++------- .../feature/send/addsend/AddSendViewModel.kt | 73 ++++++++++-- .../addsend/util/AddSendStateExtensions.kt | 22 ++-- .../send/addsend/util/SendViewExtensions.kt | 11 +- .../feature/send/addsend/AddSendScreenTest.kt | 14 ++- .../send/addsend/AddSendViewModelTest.kt | 106 +++++++++++++++++- .../util/AddSendStateExtensionsTest.kt | 20 ++-- .../addsend/util/SendViewExtensionsTest.kt | 22 +++- 8 files changed, 290 insertions(+), 77 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendContent.kt index 7efa7f26de..ce33ae74e7 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendContent.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendContent.kt @@ -116,44 +116,67 @@ fun AddSendContent( .padding(horizontal = 16.dp), ) Spacer(modifier = Modifier.height(16.dp)) - Text( - modifier = Modifier.align(Alignment.CenterHorizontally), - text = stringResource(id = R.string.no_file_chosen), - color = MaterialTheme.colorScheme.onSurfaceVariant, - style = MaterialTheme.typography.bodySmall, - ) - Spacer(modifier = Modifier.height(8.dp)) - BitwardenFilledTonalButton( - label = stringResource(id = R.string.choose_file), - onClick = { - if (permissionsManager.checkPermission(Manifest.permission.CAMERA)) { - addSendHandlers.onChooseFileClick(true) - } else { - chooseFileCameraPermissionLauncher.launch(Manifest.permission.CAMERA) - } - }, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp), - ) - Spacer(modifier = Modifier.height(4.dp)) - Text( - text = stringResource(id = R.string.max_file_size), - color = MaterialTheme.colorScheme.onSurfaceVariant, - style = MaterialTheme.typography.bodySmall, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 32.dp), - ) - Spacer(modifier = Modifier.height(16.dp)) - Text( - text = stringResource(id = R.string.type_file_info), - color = MaterialTheme.colorScheme.onSurfaceVariant, - style = MaterialTheme.typography.bodySmall, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp), - ) + if (isAddMode) { + Text( + modifier = Modifier.align(Alignment.CenterHorizontally), + text = type.name ?: stringResource(id = R.string.no_file_chosen), + color = MaterialTheme.colorScheme.onSurfaceVariant, + style = MaterialTheme.typography.bodySmall, + ) + Spacer(modifier = Modifier.height(8.dp)) + BitwardenFilledTonalButton( + label = stringResource(id = R.string.choose_file), + onClick = { + if (permissionsManager.checkPermission(Manifest.permission.CAMERA)) { + addSendHandlers.onChooseFileClick(true) + } else { + chooseFileCameraPermissionLauncher.launch( + Manifest.permission.CAMERA, + ) + } + }, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp), + ) + Spacer(modifier = Modifier.height(4.dp)) + Text( + text = stringResource(id = R.string.max_file_size), + color = MaterialTheme.colorScheme.onSurfaceVariant, + style = MaterialTheme.typography.bodySmall, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 32.dp), + ) + Spacer(modifier = Modifier.height(16.dp)) + Text( + text = stringResource(id = R.string.type_file_info), + color = MaterialTheme.colorScheme.onSurfaceVariant, + style = MaterialTheme.typography.bodySmall, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp), + ) + } else { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp), + ) { + Text( + text = type.name.orEmpty(), + color = MaterialTheme.colorScheme.onSurface, + style = MaterialTheme.typography.bodyLarge, + modifier = Modifier.weight(1f), + ) + Spacer(modifier = Modifier.width(8.dp)) + Text( + text = type.displaySize.orEmpty(), + color = MaterialTheme.colorScheme.onSurfaceVariant, + style = MaterialTheme.typography.bodyMedium, + ) + } + } } is AddSendState.ViewState.Content.SendType.Text -> { diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt index b871bdf685..262540bcda 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.ui.tools.feature.send.addsend +import android.net.Uri import android.os.Parcelable import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope @@ -32,6 +33,7 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize import java.time.Clock import java.time.ZonedDateTime @@ -40,6 +42,11 @@ import javax.inject.Inject private const val KEY_STATE = "state" +/** + * The maximum size an upload-able file is allowed to be (100 MiB). + */ +private const val MAX_FILE_SIZE_BYTES: Long = 100 * 1024 * 1024 + /** * View model for the new send screen. */ @@ -53,6 +60,7 @@ class AddSendViewModel @Inject constructor( private val environmentRepo: EnvironmentRepository, private val vaultRepo: VaultRepository, ) : BaseViewModel( + // We load the state from the savedStateHandle for testing purposes. initialState = savedStateHandle[KEY_STATE] ?: run { val addSendType = AddSendArgs(savedStateHandle).sendAddType AddSendState( @@ -93,10 +101,6 @@ class AddSendViewModel @Inject constructor( ) { init { - stateFlow - .onEach { savedStateHandle[KEY_STATE] = it } - .launchIn(viewModelScope) - when (val addSendType = state.addSendType) { AddSendType.AddItem -> Unit is AddSendType.EditItem -> { @@ -357,8 +361,13 @@ class AddSendViewModel @Inject constructor( } private fun handeFileChose(action: AddSendAction.FileChoose) { - // TODO: Process the chosen file (BIT-493) - sendEvent(AddSendEvent.ShowToast("Not Yet Implemented".asText())) + updateFileContent { + it.copy( + uri = action.fileData.uri, + name = action.fileData.fileName, + sizeBytes = action.fileData.sizeBytes, + ) + } } private fun handleRemovePasswordClick() { @@ -427,6 +436,7 @@ class AddSendViewModel @Inject constructor( updateCommonContent { it.copy(expirationDate = null) } } + @Suppress("LongMethod") private fun handleSaveClick() { onContent { content -> if (content.common.name.isBlank()) { @@ -442,6 +452,34 @@ class AddSendViewModel @Inject constructor( } return@onContent } + if (content.isFileType) { + val fileType = content.selectedType as AddSendState.ViewState.Content.SendType.File + if (fileType.name.isNullOrBlank()) { + mutableStateFlow.update { + it.copy( + dialogState = AddSendState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.validation_field_required.asText( + R.string.file.asText(), + ), + ), + ) + } + return@onContent + } + if ((fileType.sizeBytes ?: 0) > MAX_FILE_SIZE_BYTES) { + // Must be under 100 MB + mutableStateFlow.update { + it.copy( + dialogState = AddSendState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.max_file_size.asText(), + ), + ) + } + return@onContent + } + } mutableStateFlow.update { it.copy( dialogState = AddSendState.DialogState.Loading( @@ -452,9 +490,12 @@ class AddSendViewModel @Inject constructor( viewModelScope.launch { when (val addSendType = state.addSendType) { AddSendType.AddItem -> { + val fileType = content + .selectedType + as? AddSendState.ViewState.Content.SendType.File val result = vaultRepo.createSend( sendView = content.toSendView(clock), - fileUri = null, + fileUri = fileType?.uri, ) sendAction(AddSendAction.Internal.CreateSendResultReceive(result)) } @@ -494,7 +535,14 @@ class AddSendViewModel @Inject constructor( return } updateContent { - it.copy(selectedType = AddSendState.ViewState.Content.SendType.File) + it.copy( + selectedType = AddSendState.ViewState.Content.SendType.File( + uri = null, + name = null, + displaySize = null, + sizeBytes = null, + ), + ) } } @@ -658,6 +706,8 @@ data class AddSendState( */ @Parcelize data class Common( + @IgnoredOnParcel + val originalSendView: SendView? = null, val name: String, val currentAccessCount: Int?, val maxAccessCount: Int?, @@ -683,7 +733,12 @@ data class AddSendState( * Sending a file. */ @Parcelize - data object File : SendType() + data class File( + val uri: Uri?, + val name: String?, + val displaySize: String?, + val sizeBytes: Long?, + ) : SendType() /** * Sending text. diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/AddSendStateExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/AddSendStateExtensions.kt index 9185967040..9cce5b6f8d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/AddSendStateExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/AddSendStateExtensions.kt @@ -4,6 +4,7 @@ import com.bitwarden.core.SendFileView import com.bitwarden.core.SendTextView import com.bitwarden.core.SendType import com.bitwarden.core.SendView +import com.x8bit.bitwarden.ui.platform.base.util.orNullIfBlank import com.x8bit.bitwarden.ui.tools.feature.send.addsend.AddSendState import java.time.Clock @@ -14,12 +15,12 @@ fun AddSendState.ViewState.Content.toSendView( clock: Clock, ): SendView = SendView( - id = null, - accessId = null, + id = common.originalSendView?.id, + accessId = common.originalSendView?.accessId, name = common.name, - notes = common.noteInput, - key = null, - newPassword = common.passwordInput.takeUnless { it.isBlank() }, + notes = common.noteInput.orNullIfBlank(), + key = common.originalSendView?.key, + newPassword = common.passwordInput.orNullIfBlank(), hasPassword = false, type = selectedType.toSendType(), file = toSendFileView(), @@ -35,18 +36,17 @@ fun AddSendState.ViewState.Content.toSendView( private fun AddSendState.ViewState.Content.SendType.toSendType(): SendType = when (this) { - AddSendState.ViewState.Content.SendType.File -> SendType.FILE + is AddSendState.ViewState.Content.SendType.File -> SendType.FILE is AddSendState.ViewState.Content.SendType.Text -> SendType.TEXT } private fun AddSendState.ViewState.Content.toSendFileView(): SendFileView? = (this.selectedType as? AddSendState.ViewState.Content.SendType.File)?.let { - // TODO: Add support for these properties in order to save a file (BIT-1085) SendFileView( - id = "", - fileName = "", - size = "", - sizeName = "", + id = null, + fileName = it.name.orEmpty(), + size = null, + sizeName = null, ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/SendViewExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/SendViewExtensions.kt index 1f12d93264..27894554d7 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/SendViewExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/SendViewExtensions.kt @@ -16,6 +16,7 @@ fun SendView.toViewState( ): AddSendState.ViewState.Content = AddSendState.ViewState.Content( common = AddSendState.ViewState.Content.Common( + originalSendView = this, name = this.name, currentAccessCount = this.accessCount.toInt(), maxAccessCount = this.maxAccessCount?.toInt(), @@ -38,6 +39,14 @@ fun SendView.toViewState( ) } - SendType.FILE -> AddSendState.ViewState.Content.SendType.File + SendType.FILE -> { + val fileView = requireNotNull(this.file) + AddSendState.ViewState.Content.SendType.File( + uri = null, + name = fileView.fileName, + displaySize = fileView.sizeName, + sizeBytes = null, + ) + } }, ) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendScreenTest.kt index fbe7a71ba6..57d3f9ad46 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendScreenTest.kt @@ -336,7 +336,12 @@ class AddSendScreenTest : BaseComposeTest() { permissionsManager.checkPermissionResult = true mutableStateFlow.value = DEFAULT_STATE.copy( viewState = DEFAULT_VIEW_STATE.copy( - selectedType = AddSendState.ViewState.Content.SendType.File, + selectedType = AddSendState.ViewState.Content.SendType.File( + name = null, + displaySize = null, + sizeBytes = null, + uri = null, + ), ), ) composeTestRule @@ -357,7 +362,12 @@ class AddSendScreenTest : BaseComposeTest() { permissionsManager.getPermissionsResult = false mutableStateFlow.value = DEFAULT_STATE.copy( viewState = DEFAULT_VIEW_STATE.copy( - selectedType = AddSendState.ViewState.Content.SendType.File, + selectedType = AddSendState.ViewState.Content.SendType.File( + name = null, + displaySize = null, + sizeBytes = null, + uri = null, + ), ), ) composeTestRule diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModelTest.kt index efcc806c85..bf6bfc9e2e 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModelTest.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.ui.tools.feature.send.addsend +import android.net.Uri import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test import com.bitwarden.core.SendView @@ -18,6 +19,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult import com.x8bit.bitwarden.data.vault.repository.model.UpdateSendResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest import com.x8bit.bitwarden.ui.platform.base.util.asText +import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager import com.x8bit.bitwarden.ui.tools.feature.send.addsend.model.AddSendType import com.x8bit.bitwarden.ui.tools.feature.send.addsend.util.toSendView import com.x8bit.bitwarden.ui.tools.feature.send.addsend.util.toViewState @@ -277,6 +279,63 @@ class AddSendViewModelTest : BaseViewModelTest() { ) } + @Test + fun `SaveClick with file missing should show error dialog`() { + val initialState = DEFAULT_STATE.copy( + viewState = DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON_STATE.copy(name = "test"), + selectedType = AddSendState.ViewState.Content.SendType.File( + uri = null, + name = null, + displaySize = null, + sizeBytes = null, + ), + ), + ) + val viewModel = createViewModel(initialState) + + viewModel.trySendAction(AddSendAction.SaveClick) + + assertEquals( + initialState.copy( + dialogState = AddSendState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.validation_field_required.asText(R.string.file.asText()), + ), + ), + viewModel.stateFlow.value, + ) + } + + @Test + fun `SaveClick with file too large should show error dialog`() { + val initialState = DEFAULT_STATE.copy( + viewState = DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON_STATE.copy(name = "test"), + selectedType = AddSendState.ViewState.Content.SendType.File( + uri = mockk(), + name = "test.png", + displaySize = null, + // Max size is 104857600 + sizeBytes = 104857601, + ), + ), + ) + val viewModel = createViewModel(initialState) + + viewModel.trySendAction(AddSendAction.SaveClick) + + assertEquals( + initialState.copy( + dialogState = AddSendState.DialogState.Error( + title = R.string.an_error_has_occurred.asText(), + message = R.string.max_file_size.asText(), + ), + ), + viewModel.stateFlow.value, + ) + } + @Test fun `CopyLinkClick with nonnull sendUrl should copy to clipboard`() { val sendUrl = "www.test.com/send-stuff" @@ -584,11 +643,41 @@ class AddSendViewModelTest : BaseViewModelTest() { @Test fun `FileChose should emit ShowToast`() = runTest { - val viewModel = createViewModel() - viewModel.eventFlow.test { - viewModel.trySendAction(AddSendAction.FileChoose(fileData = mockk())) - assertEquals(AddSendEvent.ShowToast("Not Yet Implemented".asText()), awaitItem()) - } + val initialState = DEFAULT_STATE.copy( + viewState = DEFAULT_VIEW_STATE.copy( + selectedType = AddSendState.ViewState.Content.SendType.File( + uri = null, + name = null, + displaySize = null, + sizeBytes = null, + ), + ), + ) + val fileName = "test.png" + val uri = mockk() + val size = 50L + val fileData = IntentManager.FileData( + fileName = fileName, + uri = uri, + sizeBytes = size, + ) + val viewModel = createViewModel(initialState) + + viewModel.trySendAction(AddSendAction.FileChoose(fileData = fileData)) + + assertEquals( + initialState.copy( + viewState = DEFAULT_VIEW_STATE.copy( + selectedType = AddSendState.ViewState.Content.SendType.File( + uri = uri, + name = fileName, + displaySize = null, + sizeBytes = size, + ), + ), + ), + viewModel.stateFlow.value, + ) } @Test @@ -606,7 +695,12 @@ class AddSendViewModelTest : BaseViewModelTest() { val viewModel = createViewModel() val premiumUserState = DEFAULT_STATE.copy(isPremiumUser = true) val expectedViewState = DEFAULT_VIEW_STATE.copy( - selectedType = AddSendState.ViewState.Content.SendType.File, + selectedType = AddSendState.ViewState.Content.SendType.File( + name = null, + displaySize = null, + sizeBytes = null, + uri = null, + ), ) // Make sure we are a premium user mutableUserStateFlow.tryEmit( diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/AddSendStateExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/AddSendStateExtensionsTest.kt index 337080c545..4c220061ff 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/AddSendStateExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/AddSendStateExtensionsTest.kt @@ -1,7 +1,7 @@ package com.x8bit.bitwarden.ui.tools.feature.send.addsend.util -import com.bitwarden.core.SendFileView import com.bitwarden.core.SendType +import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockFileView import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSendView import com.x8bit.bitwarden.ui.tools.feature.send.addsend.AddSendState import org.junit.jupiter.api.Assertions.assertEquals @@ -21,17 +21,23 @@ class AddSendStateExtensionsTest { key = null, accessCount = 0U, text = null, - file = SendFileView( - id = "", - fileName = "", - size = "", - sizeName = "", + file = createMockFileView(number = 1).copy( + id = null, + size = null, + sizeName = null, ), hasPassword = false, ) val result = DEFAULT_VIEW_STATE - .copy(selectedType = AddSendState.ViewState.Content.SendType.File) + .copy( + selectedType = AddSendState.ViewState.Content.SendType.File( + name = "mockFileName-1", + displaySize = "mockSizeName-1", + sizeBytes = 1, + uri = null, + ), + ) .toSendView(FIXED_CLOCK) assertEquals(sendView, result) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/SendViewExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/SendViewExtensionsTest.kt index 3f9e17fff7..b7cf8a0db4 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/SendViewExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/util/SendViewExtensionsTest.kt @@ -21,7 +21,12 @@ class SendViewExtensionsTest { baseWebSendUrl = "www.test.com/", ) - assertEquals(DEFAULT_STATE, result) + assertEquals( + DEFAULT_STATE.copy( + common = DEFAULT_COMMON.copy(originalSendView = sendView), + ), + result, + ) } @Test @@ -33,7 +38,13 @@ class SendViewExtensionsTest { baseWebSendUrl = "www.test.com/", ) - assertEquals(DEFAULT_STATE.copy(selectedType = DEFAULT_TEXT_TYPE), result) + assertEquals( + DEFAULT_STATE.copy( + common = DEFAULT_COMMON.copy(originalSendView = sendView), + selectedType = DEFAULT_TEXT_TYPE, + ), + result, + ) } } @@ -70,7 +81,12 @@ private val DEFAULT_TEXT_TYPE: AddSendState.ViewState.Content.SendType.Text = ) private val DEFAULT_FILE_TYPE: AddSendState.ViewState.Content.SendType.File = - AddSendState.ViewState.Content.SendType.File + AddSendState.ViewState.Content.SendType.File( + name = "mockFileName-1", + displaySize = "mockSizeName-1", + sizeBytes = null, + uri = null, + ) private val DEFAULT_STATE: AddSendState.ViewState.Content = AddSendState.ViewState.Content(