Skip to content

Commit

Permalink
Merge branch 'report-a-problem-input-not-kept-across-navigation-droid…
Browse files Browse the repository at this point in the history
…-508'
  • Loading branch information
albin-mullvad committed Nov 17, 2023
2 parents 91b71ee + a3502f5 commit 1cd0cbc
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Text
import androidx.compose.material3.TextField
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
Expand All @@ -37,6 +33,7 @@ import net.mullvad.mullvadvpn.compose.component.NavigateBackIconButton
import net.mullvad.mullvadvpn.compose.component.ScaffoldWithMediumTopBar
import net.mullvad.mullvadvpn.compose.dialog.ReportProblemNoEmailDialog
import net.mullvad.mullvadvpn.compose.textfield.mullvadWhiteTextFieldColors
import net.mullvad.mullvadvpn.compose.util.SecureScreenWhileInView
import net.mullvad.mullvadvpn.dataproxy.SendProblemReportResult
import net.mullvad.mullvadvpn.lib.theme.AppTheme
import net.mullvad.mullvadvpn.lib.theme.Dimens
Expand Down Expand Up @@ -94,16 +91,15 @@ fun ReportProblemScreen(
onDismissNoEmailDialog: () -> Unit = {},
onClearSendResult: () -> Unit = {},
onNavigateToViewLogs: () -> Unit = {},
updateEmail: (String) -> Unit = {},
updateDescription: (String) -> Unit = {},
onBackClick: () -> Unit = {}
) {
var email by rememberSaveable { mutableStateOf("") }
var description by rememberSaveable { mutableStateOf("") }

// Dialog to show confirm if no email was added
if (uiState.showConfirmNoEmail) {
ReportProblemNoEmailDialog(
onDismiss = onDismissNoEmailDialog,
onConfirm = { onSendReport(email, description) }
onConfirm = { onSendReport(uiState.email, uiState.description) }
)
}

Expand All @@ -124,7 +120,10 @@ fun ReportProblemScreen(
when (uiState.sendingState) {
SendingReportUiState.Sending -> SendingContent()
is SendingReportUiState.Error ->
ErrorContent({ onSendReport(email, description) }, onClearSendResult)
ErrorContent(
{ onSendReport(uiState.email, uiState.description) },
onClearSendResult
)
is SendingReportUiState.Success -> SentContent(uiState.sendingState)
}
return@ScaffoldWithMediumTopBar
Expand All @@ -146,8 +145,8 @@ fun ReportProblemScreen(

TextField(
modifier = Modifier.fillMaxWidth(),
value = email,
onValueChange = { email = it },
value = uiState.email,
onValueChange = updateEmail,
maxLines = 1,
singleLine = true,
placeholder = { Text(text = stringResource(id = R.string.user_email_hint)) },
Expand All @@ -156,8 +155,8 @@ fun ReportProblemScreen(

TextField(
modifier = Modifier.fillMaxWidth().weight(1f),
value = description,
onValueChange = { description = it },
value = uiState.description,
onValueChange = updateDescription,
placeholder = { Text(stringResource(R.string.user_message_hint)) },
colors = mullvadWhiteTextFieldColors()
)
Expand All @@ -169,8 +168,8 @@ fun ReportProblemScreen(
)
Spacer(modifier = Modifier.height(Dimens.buttonSpacing))
VariantButton(
onClick = { onSendReport(email, description) },
isEnabled = description.isNotEmpty(),
onClick = { onSendReport(uiState.email, uiState.description) },
isEnabled = uiState.description.isNotEmpty(),
text = stringResource(id = R.string.send)
)
}
Expand All @@ -193,6 +192,7 @@ private fun ColumnScope.SendingContent() {

@Composable
private fun ColumnScope.SentContent(sendingState: SendingReportUiState.Success) {
SecureScreenWhileInView()
Icon(
painter = painterResource(id = R.drawable.icon_success),
contentDescription = stringResource(id = R.string.sent),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ sealed interface SendProblemReportResult {
}
}

data class UserReport(val email: String?, val message: String)
data class UserReport(val email: String?, val description: String)

class MullvadProblemReport(context: Context, val dispatcher: CoroutineDispatcher = Dispatchers.IO) {

Expand Down Expand Up @@ -49,7 +49,7 @@ class MullvadProblemReport(context: Context, val dispatcher: CoroutineDispatcher
withContext(dispatcher) {
sendProblemReport(
userReport.email ?: "",
userReport.message,
userReport.description,
logsPath.absolutePath,
cacheDirectory.absolutePath
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import net.mullvad.mullvadvpn.repository.ChangelogRepository
import net.mullvad.mullvadvpn.repository.DeviceRepository
import net.mullvad.mullvadvpn.repository.InAppNotificationController
import net.mullvad.mullvadvpn.repository.PrivacyDisclaimerRepository
import net.mullvad.mullvadvpn.repository.ProblemReportRepository
import net.mullvad.mullvadvpn.repository.SettingsRepository
import net.mullvad.mullvadvpn.ui.serviceconnection.RelayListListener
import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager
Expand Down Expand Up @@ -116,6 +117,8 @@ val uiModule = module {
}
}

single { ProblemReportRepository() }

// View models
viewModel { AccountViewModel(get(), get(), get(), get()) }
viewModel {
Expand All @@ -131,7 +134,7 @@ val uiModule = module {
viewModel { VoucherDialogViewModel(get(), get()) }
viewModel { VpnSettingsViewModel(get(), get(), get(), get(), get()) }
viewModel { WelcomeViewModel(get(), get(), get(), get()) }
viewModel { ReportProblemViewModel(get()) }
viewModel { ReportProblemViewModel(get(), get()) }
viewModel { ViewLogsViewModel(get()) }
viewModel { OutOfTimeViewModel(get(), get(), get(), get()) }
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package net.mullvad.mullvadvpn.repository

import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import net.mullvad.mullvadvpn.dataproxy.UserReport

class ProblemReportRepository {
private val _problemReport = MutableStateFlow(UserReport("", ""))
val problemReport: StateFlow<UserReport> = _problemReport.asStateFlow()

fun setEmail(email: String) {
_problemReport.value = _problemReport.value.copy(email = email)
}

fun setDescription(description: String) {
_problemReport.value = _problemReport.value.copy(description = description)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ class ProblemReportFragment : BaseFragment() {
onSendReport = { email, description -> vm.sendReport(email, description) },
onDismissNoEmailDialog = vm::dismissConfirmNoEmail,
onClearSendResult = vm::clearSendResult,
onNavigateToViewLogs = { showLogs() }
onNavigateToViewLogs = { showLogs() },
updateEmail = vm::updateEmail,
updateDescription = vm::updateDescription
) {
activity?.onBackPressed()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,21 @@ import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.async
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import net.mullvad.mullvadvpn.constant.MINIMUM_LOADING_TIME_MILLIS
import net.mullvad.mullvadvpn.dataproxy.MullvadProblemReport
import net.mullvad.mullvadvpn.dataproxy.SendProblemReportResult
import net.mullvad.mullvadvpn.dataproxy.UserReport
import net.mullvad.mullvadvpn.repository.ProblemReportRepository

data class ReportProblemUiState(
val showConfirmNoEmail: Boolean = false,
val sendingState: SendingReportUiState? = null
val sendingState: SendingReportUiState? = null,
val email: String = "",
val description: String = "",
)

sealed interface SendingReportUiState {
Expand All @@ -26,11 +30,28 @@ sealed interface SendingReportUiState {
data class Error(val error: SendProblemReportResult.Error) : SendingReportUiState
}

class ReportProblemViewModel(private val mullvadProblemReporter: MullvadProblemReport) :
ViewModel() {

private val _uiState = MutableStateFlow(ReportProblemUiState())
val uiState = _uiState.asStateFlow()
class ReportProblemViewModel(
private val mullvadProblemReporter: MullvadProblemReport,
private val problemReportRepository: ProblemReportRepository
) : ViewModel() {

private val showConfirmNoEmail = MutableStateFlow(false)
private val sendingState: MutableStateFlow<SendingReportUiState?> = MutableStateFlow(null)

val uiState =
combine(
showConfirmNoEmail,
sendingState,
problemReportRepository.problemReport,
) { showConfirmNoEmail, pendingState, userReport ->
ReportProblemUiState(
showConfirmNoEmail = showConfirmNoEmail,
sendingState = pendingState,
email = userReport.email ?: "",
description = userReport.description,
)
}
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), ReportProblemUiState())

fun sendReport(
email: String,
Expand All @@ -40,31 +61,42 @@ class ReportProblemViewModel(private val mullvadProblemReporter: MullvadProblemR
val userEmail = email.trim()
val nullableEmail = if (email.isEmpty()) null else userEmail
if (shouldShowConfirmNoEmail(nullableEmail)) {
_uiState.update { it.copy(showConfirmNoEmail = true) }
showConfirmNoEmail.tryEmit(true)
} else {
_uiState.update {
it.copy(sendingState = SendingReportUiState.Sending, showConfirmNoEmail = false)
}
sendingState.tryEmit(SendingReportUiState.Sending)
showConfirmNoEmail.tryEmit(false)

// Ensure we show loading for at least MINIMUM_LOADING_TIME_MILLIS
val deferredResult = async {
mullvadProblemReporter.sendReport(UserReport(nullableEmail, description))
}
delay(MINIMUM_LOADING_TIME_MILLIS)

_uiState.update {
it.copy(sendingState = deferredResult.await().toUiResult(nullableEmail))
val result = deferredResult.await()
// Clear saved problem report if report was sent successfully
if (result is SendProblemReportResult.Success) {
problemReportRepository.setEmail("")
problemReportRepository.setDescription("")
}
sendingState.tryEmit(deferredResult.await().toUiResult(nullableEmail))
}
}
}

fun clearSendResult() {
_uiState.update { it.copy(sendingState = null) }
sendingState.tryEmit(null)
}

fun dismissConfirmNoEmail() {
_uiState.update { it.copy(showConfirmNoEmail = false) }
showConfirmNoEmail.tryEmit(false)
}

fun updateEmail(email: String) {
problemReportRepository.setEmail(email)
}

fun updateDescription(description: String) {
problemReportRepository.setDescription(description)
}

private fun shouldShowConfirmNoEmail(userEmail: String?): Boolean =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ import app.cash.turbine.test
import io.mockk.MockKAnnotations
import io.mockk.coEvery
import io.mockk.impl.annotations.MockK
import io.mockk.verify
import kotlin.test.assertEquals
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
import net.mullvad.mullvadvpn.dataproxy.MullvadProblemReport
import net.mullvad.mullvadvpn.dataproxy.SendProblemReportResult
import net.mullvad.mullvadvpn.dataproxy.UserReport
import net.mullvad.mullvadvpn.lib.common.test.TestCoroutineRule
import net.mullvad.mullvadvpn.repository.ProblemReportRepository
import org.junit.After
import org.junit.Before
import org.junit.Rule
Expand All @@ -20,14 +24,18 @@ class ReportProblemModelTest {
@get:Rule val testCoroutineRule = TestCoroutineRule()

@MockK private lateinit var mockMullvadProblemReport: MullvadProblemReport
@MockK(relaxed = true) private lateinit var mockProblemReportRepository: ProblemReportRepository

private val problemReportFlow = MutableStateFlow(UserReport("", ""))

private lateinit var viewModel: ReportProblemViewModel

@Before
fun setUp() {
MockKAnnotations.init(this)
coEvery { mockMullvadProblemReport.collectLogs() } returns true
viewModel = ReportProblemViewModel(mockMullvadProblemReport)
coEvery { mockProblemReportRepository.problemReport } returns problemReportFlow
viewModel = ReportProblemViewModel(mockMullvadProblemReport, mockProblemReportRepository)
}

@After
Expand Down Expand Up @@ -114,4 +122,43 @@ class ReportProblemModelTest {
)
}
}

@Test
fun testUpdateEmail() = runTest {
// Arrange
val email = "[email protected]"

// Act
viewModel.updateEmail(email)

// Assert
verify { mockProblemReportRepository.setEmail(email) }
}

@Test
fun testUpdateDescription() = runTest {
// Arrange
val description = "My description"

// Act
viewModel.updateDescription(description)

// Assert
verify { mockProblemReportRepository.setDescription(description) }
}

@Test
fun testUpdateProblemReport() = runTest {
// Arrange
val userReport = UserReport("[email protected]", "My description")

// Act, Assert
viewModel.uiState.test {
awaitItem()
problemReportFlow.value = userReport
val result = awaitItem()
assertEquals(userReport.email, result.email)
assertEquals(userReport.description, result.description)
}
}
}

0 comments on commit 1cd0cbc

Please sign in to comment.