From e69920064ed261af335e9913202fb7693d2bc17c Mon Sep 17 00:00:00 2001
From: Fabian-K <fabian.kajzar@sap.com>
Date: Sat, 13 Jun 2020 10:10:10 +0200
Subject: [PATCH] Submission Improvement (#438)

* Warn Others Fragment
- control button enabled and progressbar visible via databinding & formatter
- remove requestPermissionToShareKeys() from onResume which causes multiple submissions to be triggered
- remove unnecessary state submissionRequested & submissionFailed
- add rudimentary error handling to onFailure of exposure notification API <- needs to be confirmed!!

SubmissionViewModel
- inline executeRequestWithStateForEvent
- changed submissionState from Event<ApiRequestState> to ApiRequestState to enable use of formatter
- use postValue to update from background thread

misc:
- remove unused formatShowTanCharacterError method

* Warn Others
- adjust onFailure handling
---
 ...nalExposureNotificationPermissionHelper.kt |  5 --
 .../ui/settings/SettingsTracingFragment.kt    |  1 -
 ...ssionResultPositiveOtherWarningFragment.kt | 42 +++-------
 .../ui/viewmodel/SubmissionViewModel.kt       | 83 ++++++++++---------
 .../formatter/FormatterSubmissionHelper.kt    | 11 +--
 ...ment_submission_positive_other_warning.xml | 16 +++-
 6 files changed, 74 insertions(+), 84 deletions(-)

diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/InternalExposureNotificationPermissionHelper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/InternalExposureNotificationPermissionHelper.kt
index 8891ecf4c..8f323857c 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/InternalExposureNotificationPermissionHelper.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/nearby/InternalExposureNotificationPermissionHelper.kt
@@ -69,11 +69,6 @@ class InternalExposureNotificationPermissionHelper(
                     ResolutionRequestCodes.REQUEST_CODE_START_EXPOSURE_NOTIFICATION.code
                 )
             } catch (exception: Exception) {
-                exception.report(
-                    ExceptionCategory.EXPOSURENOTIFICATION,
-                    TAG,
-                    null
-                )
                 returnError(exception)
             }
         }
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/settings/SettingsTracingFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/settings/SettingsTracingFragment.kt
index c2390c539..2b41203d1 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/settings/SettingsTracingFragment.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/settings/SettingsTracingFragment.kt
@@ -89,7 +89,6 @@ class SettingsTracingFragment : Fragment(),
 
     override fun onFailure(exception: Exception?) {
         tracingViewModel.refreshIsTracingEnabled()
-        exception?.report(ExceptionCategory.EXPOSURENOTIFICATION)
     }
 
     private fun setButtonOnClickListener() {
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/SubmissionResultPositiveOtherWarningFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/SubmissionResultPositiveOtherWarningFragment.kt
index 92c15d7fc..663e77847 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/SubmissionResultPositiveOtherWarningFragment.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/SubmissionResultPositiveOtherWarningFragment.kt
@@ -8,6 +8,7 @@ import android.view.ViewGroup
 import android.view.accessibility.AccessibilityEvent
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.activityViewModels
+import androidx.lifecycle.Observer
 import androidx.navigation.fragment.findNavController
 import com.google.android.gms.nearby.exposurenotification.TemporaryExposureKey
 import de.rki.coronawarnapp.R
@@ -35,8 +36,6 @@ class SubmissionResultPositiveOtherWarningFragment : Fragment(),
 
     private var _binding: FragmentSubmissionPositiveOtherWarningBinding? = null
     private val binding: FragmentSubmissionPositiveOtherWarningBinding get() = _binding!!
-    private var submissionRequested = false
-    private var submissionFailed = false
     private lateinit var internalExposureNotificationPermissionHelper:
             InternalExposureNotificationPermissionHelper
 
@@ -44,20 +43,6 @@ class SubmissionResultPositiveOtherWarningFragment : Fragment(),
         super.onResume()
         binding.submissionPositiveOtherPrivacyContainer.sendAccessibilityEvent(AccessibilityEvent.TYPE_ANNOUNCEMENT)
         tracingViewModel.refreshIsTracingEnabled()
-        if (submissionRequested && !submissionFailed) {
-            internalExposureNotificationPermissionHelper.requestPermissionToShareKeys()
-        }
-    }
-
-    override fun onKeySharePermissionGranted(keys: List<TemporaryExposureKey>) {
-        super.onKeySharePermissionGranted(keys)
-        submissionViewModel.submitDiagnosisKeys(keys)
-    }
-
-    override fun onFailure(exception: Exception?) {
-        binding.submissionPositiveOtherWarningButtonNext.isEnabled = true
-        binding.submissionPositiveOtherWarningSpinner.visibility = View.GONE
-        submissionFailed = true
     }
 
     override fun onCreateView(
@@ -68,6 +53,7 @@ class SubmissionResultPositiveOtherWarningFragment : Fragment(),
         internalExposureNotificationPermissionHelper =
             InternalExposureNotificationPermissionHelper(this, this)
         _binding = FragmentSubmissionPositiveOtherWarningBinding.inflate(inflater)
+        binding.submissionViewModel = submissionViewModel
         binding.lifecycleOwner = this
         return binding.root
     }
@@ -141,17 +127,7 @@ class SubmissionResultPositiveOtherWarningFragment : Fragment(),
             DialogHelper.showDialog(buildErrorDialog(it))
         })
 
-        submissionViewModel.submissionState.observeEvent(viewLifecycleOwner, {
-            binding.submissionPositiveOtherWarningButtonNext.isEnabled = when (it) {
-                ApiRequestState.STARTED -> false
-                else -> true
-            }
-
-            binding.submissionPositiveOtherWarningSpinner.visibility = when (it) {
-                ApiRequestState.STARTED -> View.VISIBLE
-                else -> View.GONE
-            }
-
+        submissionViewModel.submissionState.observe(viewLifecycleOwner, Observer {
             if (it == ApiRequestState.SUCCESS) {
                 findNavController().doNavigate(
                     SubmissionResultPositiveOtherWarningFragmentDirections
@@ -163,8 +139,6 @@ class SubmissionResultPositiveOtherWarningFragment : Fragment(),
 
     private fun setButtonOnClickListener() {
         binding.submissionPositiveOtherWarningButtonNext.setOnClickListener {
-            binding.submissionPositiveOtherWarningButtonNext.isEnabled = false
-            binding.submissionPositiveOtherWarningSpinner.visibility = View.VISIBLE
             initiateWarningOthers()
         }
         binding.submissionPositiveOtherWarningHeader.headerButtonBack.buttonIcon.setOnClickListener {
@@ -190,7 +164,6 @@ class SubmissionResultPositiveOtherWarningFragment : Fragment(),
             return
         }
 
-        submissionRequested = true
         internalExposureNotificationPermissionHelper.requestPermissionToShareKeys()
     }
 
@@ -200,4 +173,13 @@ class SubmissionResultPositiveOtherWarningFragment : Fragment(),
             resultCode
         )
     }
+
+    // InternalExposureNotificationPermissionHelper - callbacks
+    override fun onKeySharePermissionGranted(keys: List<TemporaryExposureKey>) {
+        super.onKeySharePermissionGranted(keys)
+        submissionViewModel.submitDiagnosisKeys(keys)
+    }
+
+    override fun onFailure(exception: Exception?) {
+    }
 }
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt
index 75fb016f7..95f2943ed 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt
@@ -28,7 +28,7 @@ class SubmissionViewModel : ViewModel() {
     private val _uiStateState = MutableLiveData(ApiRequestState.IDLE)
     private val _uiStateError = MutableLiveData<Event<CwaWebException>>(null)
 
-    private val _submissionState = MutableLiveData(Event(ApiRequestState.IDLE))
+    private val _submissionState = MutableLiveData(ApiRequestState.IDLE)
     private val _submissionError = MutableLiveData<Event<CwaWebException>>(null)
 
     val scanStatus: LiveData<Event<ScanStatus>> = _scanStatus
@@ -39,7 +39,7 @@ class SubmissionViewModel : ViewModel() {
     val uiStateState: LiveData<ApiRequestState> = _uiStateState
     val uiStateError: LiveData<Event<CwaWebException>> = _uiStateError
 
-    val submissionState: LiveData<Event<ApiRequestState>> = _submissionState
+    val submissionState: LiveData<ApiRequestState> = _submissionState
     val submissionError: LiveData<Event<CwaWebException>> = _submissionError
 
     val deviceRegistered get() = LocalData.registrationToken() != null
@@ -49,19 +49,47 @@ class SubmissionViewModel : ViewModel() {
     val deviceUiState: LiveData<DeviceUIState> =
         SubmissionRepository.deviceUIState
 
-    fun submitDiagnosisKeys(keys: List<TemporaryExposureKey>) =
-        executeRequestWithStateForEvent(
-            { SubmissionService.asyncSubmitExposureKeys(keys) },
-            _submissionState,
-            _submissionError
-        )
+    fun submitDiagnosisKeys(keys: List<TemporaryExposureKey>) = viewModelScope.launch {
+        try {
+            _submissionState.value = ApiRequestState.STARTED
+            SubmissionService.asyncSubmitExposureKeys(keys)
+            _submissionState.value = ApiRequestState.SUCCESS
+        } catch (err: CwaWebException) {
+            _submissionError.value = Event(err)
+            _submissionState.value = ApiRequestState.FAILED
+        } catch (err: TransactionException) {
+            if (err.cause is CwaWebException) {
+                _submissionError.value = Event(err.cause)
+            } else {
+                err.report(ExceptionCategory.INTERNAL)
+            }
+            _submissionState.value = ApiRequestState.FAILED
+        } catch (err: Exception) {
+            _submissionState.value = ApiRequestState.FAILED
+            err.report(ExceptionCategory.INTERNAL)
+        }
+    }
 
-    fun doDeviceRegistration() =
-        executeRequestWithStateForEvent(
-            SubmissionService::asyncRegisterDevice,
-            _registrationState,
-            _registrationError
-        )
+    fun doDeviceRegistration() = viewModelScope.launch {
+        try {
+            _registrationState.value = Event(ApiRequestState.STARTED)
+            SubmissionService.asyncRegisterDevice()
+            _registrationState.value = Event(ApiRequestState.SUCCESS)
+        } catch (err: CwaWebException) {
+            _registrationError.value = Event(err)
+            _registrationState.value = Event(ApiRequestState.FAILED)
+        } catch (err: TransactionException) {
+            if (err.cause is CwaWebException) {
+                _registrationError.value = Event(err.cause)
+            } else {
+                err.report(ExceptionCategory.INTERNAL)
+            }
+            _registrationState.value = Event(ApiRequestState.FAILED)
+        } catch (err: Exception) {
+            _registrationState.value = Event(ApiRequestState.FAILED)
+            err.report(ExceptionCategory.INTERNAL)
+        }
+    }
 
     fun refreshDeviceUIState() =
         executeRequestWithState(
@@ -109,31 +137,4 @@ class SubmissionViewModel : ViewModel() {
             }
         }
     }
-
-    private fun executeRequestWithStateForEvent(
-        apiRequest: suspend () -> Unit,
-        state: MutableLiveData<Event<ApiRequestState>>,
-        exceptionLiveData: MutableLiveData<Event<CwaWebException>>? = null
-    ) {
-        state.value = Event(ApiRequestState.STARTED)
-        viewModelScope.launch {
-            try {
-                apiRequest()
-                state.value = Event(ApiRequestState.SUCCESS)
-            } catch (err: CwaWebException) {
-                exceptionLiveData?.value = Event(err)
-                state.value = Event(ApiRequestState.FAILED)
-            } catch (err: TransactionException) {
-                if (err.cause is CwaWebException) {
-                    exceptionLiveData?.value = Event(err.cause)
-                } else {
-                    err.report(ExceptionCategory.INTERNAL)
-                }
-                state.value = Event(ApiRequestState.FAILED)
-            } catch (err: Exception) {
-                state.value = Event(ApiRequestState.FAILED)
-                err.report(ExceptionCategory.INTERNAL)
-            }
-        }
-    }
 }
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/formatter/FormatterSubmissionHelper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/formatter/FormatterSubmissionHelper.kt
index 10004cdff..5d93578b1 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/formatter/FormatterSubmissionHelper.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/formatter/FormatterSubmissionHelper.kt
@@ -21,6 +21,12 @@ fun formatTestResultSpinnerVisible(uiStateState: ApiRequestState?): Int =
 fun formatTestResultVisible(uiStateState: ApiRequestState?): Int =
     formatVisibility(uiStateState == ApiRequestState.SUCCESS)
 
+fun formatSubmitButtonEnabled(apiRequestState: ApiRequestState) =
+    apiRequestState == ApiRequestState.IDLE || apiRequestState == ApiRequestState.FAILED
+
+fun formatSubmitSpinnerVisible(apiRequestState: ApiRequestState) =
+    formatVisibility(apiRequestState == ApiRequestState.STARTED)
+
 fun formatTestResultStatusText(uiState: DeviceUIState?): String {
     val appContext = CoronaWarnApplication.getAppContext()
     return when (uiState) {
@@ -192,8 +198,3 @@ fun formatShowRiskStatusCard(deviceUiState: DeviceUIState?): Int =
                 deviceUiState != DeviceUIState.PAIRED_POSITIVE_TELETAN &&
                 deviceUiState != DeviceUIState.SUBMITTED_FINAL
     )
-
-fun formatShowTanCharacterError(
-    charactersValid: Boolean,
-    checksumValid: Boolean
-): Int = formatVisibility(checksumValid && !charactersValid)
diff --git a/Corona-Warn-App/src/main/res/layout/fragment_submission_positive_other_warning.xml b/Corona-Warn-App/src/main/res/layout/fragment_submission_positive_other_warning.xml
index 638b1c894..92dd63fe9 100644
--- a/Corona-Warn-App/src/main/res/layout/fragment_submission_positive_other_warning.xml
+++ b/Corona-Warn-App/src/main/res/layout/fragment_submission_positive_other_warning.xml
@@ -3,11 +3,22 @@
     xmlns:app="http://schemas.android.com/apk/res-auto"
     xmlns:tools="http://schemas.android.com/tools">
 
+    <data>
+
+        <import type="de.rki.coronawarnapp.util.formatter.FormatterSubmissionHelper" />
+
+        <variable
+            name="submissionViewModel"
+            type="de.rki.coronawarnapp.ui.viewmodel.SubmissionViewModel" />
+
+    </data>
+
+
     <androidx.constraintlayout.widget.ConstraintLayout
         android:id="@+id/submission_positive_other_privacy_container"
-        android:contentDescription="@string/submission_positive_other_warning_title"
         android:layout_width="match_parent"
         android:layout_height="match_parent"
+        android:contentDescription="@string/submission_positive_other_warning_title"
         android:fillViewport="true"
         tools:context=".ui.submission.SubmissionResultPositiveOtherWarningFragment">
 
@@ -38,6 +49,7 @@
             style="@style/buttonPrimary"
             android:layout_width="@dimen/match_constraint"
             android:layout_height="wrap_content"
+            android:enabled="@{FormatterSubmissionHelper.formatSubmitButtonEnabled(submissionViewModel.submissionState)}"
             android:text="@string/submission_positive_other_warning_button"
             android:textAllCaps="true"
             app:layout_constraintBottom_toBottomOf="parent"
@@ -52,7 +64,7 @@
             android:layout_height="wrap_content"
             android:layout_marginTop="@dimen/spacing_large"
             android:indeterminate="true"
-            android:visibility="gone"
+            android:visibility="@{FormatterSubmissionHelper.formatSubmitSpinnerVisible(submissionViewModel.submissionState)}"
             app:layout_constraintBottom_toBottomOf="@+id/submission_positive_other_warning_button_next"
             app:layout_constraintEnd_toEndOf="@+id/submission_positive_other_warning_button_next"
             app:layout_constraintStart_toStartOf="@+id/submission_positive_other_warning_button_next"
-- 
GitLab