Review: Cleaning / detekt / improve test

features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt

@@ -38,8 +38,7 @@ import io.element.android.libraries.di.ApplicationContext
 import io.element.android.libraries.di.SingleIn
 import io.element.android.libraries.matrix.api.MatrixClientProvider
 import io.element.android.libraries.matrix.api.SdkMetadata
-import io.element.android.libraries.matrix.api.core.MatrixPatterns
-import io.element.android.libraries.matrix.api.core.SessionId
+import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.network.useragent.UserAgentProvider
 import io.element.android.libraries.sessionstorage.api.SessionStore
 import kotlinx.coroutines.CancellationException
@@ -82,7 +81,7 @@ class DefaultBugReporter @Inject constructor(
     private val buildMeta: BuildMeta,
     private val bugReporterUrlProvider: BugReporterUrlProvider,
     private val sdkMetadata: SdkMetadata,
-    private val matrixClientsProvider: MatrixClientProvider,
+    private val matrixClientProvider: MatrixClientProvider,
 ) : BugReporter {
     companion object {
         // filenames
@@ -149,7 +148,7 @@ class DefaultBugReporter @Inject constructor(
 
                 val sessionData = sessionStore.getLatestSession()
                 val deviceId = sessionData?.deviceId ?: "undefined"
-                val userId = sessionData?.userId ?: "undefined"
+                val userId = sessionData?.userId?.let { UserId(it) }
 
                 if (!isCancelled) {
                     // build the multi part request
@@ -157,14 +156,12 @@ class DefaultBugReporter @Inject constructor(
                         .addFormDataPart("text", bugDescription)
                         .addFormDataPart("app", context.getString(R.string.bug_report_app_name))
                         .addFormDataPart("user_agent", userAgentProvider.provide())
-                        .addFormDataPart("user_id", userId)
+                        .addFormDataPart("user_id", userId?.toString() ?: "undefined")
                         .addFormDataPart("can_contact", canContact.toString())
                         .addFormDataPart("device_id", deviceId)
                         .apply {
-                            userId.takeIf { MatrixPatterns.isUserId(it) }?.let {
-                                SessionId(it)
-                            }?.let { sessionId ->
-                                matrixClientsProvider.getOrNull(sessionId)?.let { client ->
+                            userId?.let {
+                                matrixClientProvider.getOrNull(it)?.let { client ->
                                     val curveKey = client.encryptionService().deviceCurve25519()
                                     val edKey = client.encryptionService().deviceEd25519()
                                     if (curveKey != null && edKey != null) {

features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt

@@ -135,16 +135,27 @@ class DefaultBugReporterTest {
             buildMeta = buildMeta,
             bugReporterUrlProvider = { server.url("/") },
             sdkMetadata = FakeSdkMetadata("123456789"),
-            matrixClientsProvider = FakeMatrixClientProvider(getClient = { Result.success(matrixClient) })
+            matrixClientProvider = FakeMatrixClientProvider(getClient = { Result.success(matrixClient) })
         )
 
+        val progressValues = mutableListOf<Int>()
         sut.sendBugReport(
             withDevicesLogs = true,
             withCrashLogs = true,
             withScreenshot = true,
             theBugDescription = "a bug occurred",
             canContact = true,
-            listener = null
+            listener = object : BugReporterListener {
+                override fun onUploadCancelled() {}
+
+                override fun onUploadFailed(reason: String?) {}
+
+                override fun onProgress(progress: Int) {
+                    progressValues.add(progress)
+                }
+
+                override fun onUploadSucceed() {}
+            },
         )
         val request = server.takeRequest()
 
@@ -155,12 +166,14 @@ class DefaultBugReporterTest {
             // Just use simple parsing to detect basic properties
             val regex = "form-data; name=\"(\\w*)\".*".toRegex()
             multipartReader.use {
-                while (true) {
-                    val part = multipartReader.nextPart() ?: break
-                    val contentDisposition = part.headers["Content-Disposition"] ?: continue
-                    regex.find(contentDisposition)?.groupValues?.get(1)?.let { name ->
-                        foundValues.put(name, part.body.readUtf8())
+                var part = multipartReader.nextPart()
+                while (part != null) {
+                        part.headers["Content-Disposition"]?.let { contentDisposition ->
+                        regex.find(contentDisposition)?.groupValues?.get(1)?.let { name ->
+                            foundValues.put(name, part!!.body.readUtf8())
+                        }
                     }
+                    part = multipartReader.nextPart()
                 }
             }
         }
@@ -173,6 +186,9 @@ class DefaultBugReporterTest {
         assertThat(foundValues["text"]).isEqualTo("a bug occurred")
         assertThat(foundValues["device_keys"]).isEqualTo("curve25519:CURVECURVECURVE, ed25519:EDKEYEDKEYEDKY")
 
+        // device_key now added given they are not null
+        assertThat(progressValues.size).isEqualTo(EXPECTED_NUMBER_OF_PROGRESS_VALUE + 1)
+
         server.shutdown()
     }
 
@@ -242,7 +258,7 @@ class DefaultBugReporterTest {
             buildMeta = buildMeta,
             bugReporterUrlProvider = { server.url("/") },
             sdkMetadata = FakeSdkMetadata("123456789"),
-            matrixClientsProvider = FakeMatrixClientProvider()
+            matrixClientProvider = FakeMatrixClientProvider()
         )
     }